5 Replies Latest reply on Jul 10, 2007 12:30 PM by brian.stansberry

    A timebomb in the UnlockInterceptor?

    girafy

      Hello,

      We are using the JBossCache v.1.4.1.SP3. Recently we started experiencing a locking problem: lots of TimeoutExceptions: read/write lock for ... could not be acquired after a timeout. After analyzing the source coude we have noticed the following:

      In the UnlockInterceptor there is a trace output:

      if (trace) log.trace("Attempting to release locks on current thread. Lock table is " + lock_table);


      The trace variable (log.isTraceEnabled()) is true when you use the log4j logging and configure your thresholds on appenders level (our case). So, the concatenation in the above code line does happen although the console shows no output.
      The concatenation causes the toString() method to be called on all IdentityLock objects that are contained in the lock_table. The toString() logic there is quite heavy: it gets all reader owners (there can be lots of them), adds them one by one to an ArrayList, and then adds this list into a StringBuffer which in turn causes the toString() to be invoked on every reader. So this takes time.
      Now, all of this happens before releasing a lock, so if another thread tries to acquire a lock, it'll have to wait until the current thread finishes its concatenation. And there is a snowball effect: the more threads are performing this, the less chances each individual thread (including the write lock owner) has to release its lock. And since the logic is in the finally block, even those threads that were unsuccessful in obtaining a lock, have to do this concatenation before exiting which provokes more and more threads to be slower and slower passing this bottleneck.
      We've performed a little test: 300 threads, 1 node, each thread randomly puts or reads the data from this node 30 times. Without a concatenation (when the overall debug level is set to INFO) this finishes in less than a minute and all operations are successful. When the debug level is set to DEBUG, the test can be running several hours or even produce an out of memory error, and almost all of the read/write attempts fail because of the TimeoutException.
      So we suggest to remove the above debug code from the future versions of the cache. Also, maybe it's a good idea to remove the heavy code from the toString() method to prevent such situations from occuring.

      Thank you.

        • 1. Re: A timebomb in the UnlockInterceptor?
          genman

          Explicitly set the logging level for this class. Is there something wrong with doing that?

          • 2. Re: A timebomb in the UnlockInterceptor?
            genman

            You can do this by doing
            Logger.getLogger(IdentityLock.class).setLevel(Level.OFF);

            • 3. Re: A timebomb in the UnlockInterceptor?
              girafy

              Changing the logging level is perfiectly valid. This problem is quite easy to fix once you understand what's going on. ;)
              I just wanted to let the others know that there is an issue, so they won't spend 2 days as I did to find the reason. And also to show the developers that there is a potentially dangerous code in this class.
              Even if there is no lock, it might be that the performance of the treecache for some users will become 10x times higher after changing the debug level for this class.

              Cheers.

              • 4. Re: A timebomb in the UnlockInterceptor?
                manik

                Poor performance is expected when you have logging set to such a detailed level. IMO, it is the price you pay for getting detailed information about a running system.

                Again, I expect that such slowdowns would cause timeouts, etc. to lapse.

                The correct solution - as genman suggested - is to tune your log levels, even on a per-class basis if need be.

                • 5. Re: A timebomb in the UnlockInterceptor?
                  brian.stansberry

                  Note that if you use a more recent log4j release and commons-logging 1.1, the logging framework will properly distinguish between DEBUG and TRACE.