#963 MemcachedStorage Notice if deamon is not running, incl. test

[patriksima]

No description provided.

[milo]

Is it necessary to fix it in this way? This is an advantage of Memcache to add server which may be offline. Other servers may be online and this one just loose priority.

I hope, better solution is to somehow provide access to 8th addServer() parameter where user can handle these errors.

[milo]

There is an Assert::exception() for this purpose.

[patriksima]

Cool. That's exactly what I was looking for. This code I copied from other tests. Btw. Is there any help for Nette/Tester?

[patriksima]

I cant imagine that someone uses memcache offline. What's the point? But, if so, can wrap each call addServer in try/catch block and then work with it.

[milo]

$mc->addServer('1st.domain.tld');
$mc->addServer('2nd.domain.tld'); // Offline now? Who cares...
$mc->addServer('3nd.domain.tld');

[patriksima]

The point is that the exception is better than hidden notice. Exception can be handled explicitly, notice no.

[patriksima]

I can't be sure that pull out failure_callback is the solution. you got it checked?

[milo]

My point is that offline state is a normal cache state, not a fault. Cache is not permanent data storage.

With your patch... what happend when server is online when addServer() and offline when read(), write()? Notice is emmited.

[milo]

I checked that "offline message" notice is catched by callback.

[patriksima]

Both method return NULL or FALSE if server failed.

[patriksima]

I noticed that method write($key, $data, array $dp) don`t return value. But it should IMHO.

[milo]

Yes, NULL or FALSE is returned without notice, but only when server is offline during addServer(). If gets offline between addServer() and read()/write(), Notice is still thrown.

[patriksima]

Okey. I got it. Options:

• supress notice by Debugger::$strictMode
• supress notice by @ inside both method read() and write()
• pull out failure_callback to addServer() method params
• combination of the above

[patriksima]

And I fix write() to return a value. Plus fix assertion in my test.

[dg]

@patriksima I am not using memcached and I don't know if this pull is good to merge. This #980 (comment) makes sense to me.

[patriksima]

@dg I wrote options. Which way should I go?

[milo]

I have done small investigation...

One big advantage of Memcache class is, that can be used with offline servers. Actually it works without added servers. Yes, it emits notices/warnings but works.

Adding server even if is offline is important. Memcache do statistics and prioritize servers. Exception cannot be throws due to BC (application stops working with offline server, moreover, addServer is in constructor).

Shut-up operator in read(), write() and others is not IMO good idea due to suppressing warnings too.

Pass failure callback for Memcache::addServer() is a half-good way. It catch all notices, but only host and port is passed and you never get the error message. This can be now achived by $mc->getConnection()->setServerParams(). But there is easy to make a loop when accessing Memcache from callback.

Better solution is install error handler before every memcache method calling and catch notices (see d59555a). I was afraid about performace, but on my laptop:

for ($i = 0; $i < 100000; $i++) { # 20ms
}

for ($i = 0; $i < 100000; $i++) { # 380ms
    set_error_handler(function() { restore_error_handler(); });
    restore_error_handler();
}

Question is, if handle only notices or all errors (http://svn.php.net/viewvc/pecl/memcache/trunk/memcache.c?view=markup).