We think some logs are missing when using memcache engine in shared mode.

We would like to add watchdog() calls for the following points:

  • lock() function returns FALSE: could mean that a cache table is locked forever
  • unlock() function returns FALSE: could lead to previous point
  • set() on lookup returns FALSE: could mean that the lookup value has reached more than 1MB and thus has been removed by memcache server

We will probably work on that and deliver a patch. Feel free to give your opinion on this topic.

CommentFileSizeAuthor
#9 cacherouter-missing_logs-1071508-9.patch6.65 KBatouchard
#7 cacherouter-missing_logs-1071508-7.patch6.14 KBatouchard

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Version: 6.x-1.0-rc1 » 6.x-1.x-dev

I think it's not a good idea to put core's functions into engines because cacherouter could work before full bootstrap so no DB available and no core modules are loaded... this way to BSOD because module_implements() probably not loaded too...

bpresles’s picture

bpresles commented

We thought to use watchdog() because we saw that it was already used in connect() function of engines/memcache.php (watchdog('cache', "Unable to connect to memcache server $host:$port", WATCHDOG_ERROR);).

What alternatives would you suggest? error_log()?

andypost’s picture

andypost
he/him
Primary language Russian
commented

I think it's better to introduce a method (debug?) for base class that check for full bootstrap and then uses watchdog()

sdelbosc’s picture

sdelbosc commented

You are right.

I checked drupal_bootstrap() code and added watchdog() calls on the different steps. If watchdog() is called too early during the bootstrap, module_implements() is not defined.

I suggest writing a trace() function - with the same signature as watchdog() - which calls error_log() when it is called during bootstrap and watchdog() otherwise.

My problem with this approach is that I think it should be handled by the watchdog() function provided by the core and not by a custom trace() function. Do you share the same opinion?

andypost’s picture

andypost
he/him
Primary language Russian
commented

I think a bit different - trace() is probably a good METHOD of base class but you talk about FUNCTION...

sdelbosc’s picture

sdelbosc commented

Again, you are right. If cacherouter module is updated for that, it should be to add a method in the base class and not a function.

However, my thought was that it should be up to watchdog() function of Drupal core to deal with the case where bootstrap is not finished.

atouchard’s picture

atouchard commented
StatusFileSize
new cacherouter-missing_logs-1071508-7.patch6.14 KB

We add trace() function. This function switch between watchdog() and error_log() (watchdog log format) by bootstrap is finished or not. What do you think ?

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs work

Great! This feature require a bit of settings to be used per cache-bin and reflection in other engines (but this is a different task/issue)

+++ ./cacherouter/Cache.php	2011-03-02 15:50:15.174311002 +0100
@@ -123,4 +123,61 @@ class Cache {
+    // If the bootstrap is fully done, use watchdog
+    if (function_exists("module_invoke")) {
+      watchdog($type, $message, $variables, $severity, $link);
+    }
+    // Otherwise use error_log
+    else {
+      // Prepare the fields to be logged

This logging should be disabled by default as it now. Suppose a parameter with FALSE defaults is enough.

+++ ./cacherouter/engines/memcache.php	2011-03-02 15:52:46.994311001 +0100
@@ -200,7 +224,7 @@ class memcacheCache extends Cache {
-        watchdog('cache', "Unable to connect to memcache server $host:$port", WATCHDOG_ERROR);
+        $this->trace('cache', "Unable to connect to memcache server $host:$port", WATCHDOG_ERROR);

This message should contain an $this->name to be usable

Powered by Dreditor.

atouchard’s picture

atouchard commented
StatusFileSize
new cacherouter-missing_logs-1071508-9.patch6.65 KB

An improved patch :

  • adding the size of the cache entry in log messages
  • adding global variables missing