wang-haha / memcached-session-manager

Automatically exported from code.google.com/p/memcached-session-manager
0 stars 0 forks source link

Inconsistent validity key calculation in presence of storage key perfix #210

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. setup non-sticky locking configuration with non-empty storage key prefix and 
2 memcached nodes
2. create a new session
3. turn off primary memcached node for that session

The next request should trigger msm to load the session from the remaining 
(backup) memcached node. Now the key to retrieve the validity information from 
the backup memcached node is calculated differently from the key used when 
storing the info.

This behaviour has been observed in version 1.8.2. 

I'm using the following settings in context.xml:

<Context sessionCookiePath="/">
  <Manager className="de.javakaffee.web.msm.MemcachedBackupSessionManager"
    memcachedNodes="${msm.memcachedNodes}"
    storageKeyPrefix="static:webtests,webappVersion"
    sticky="false"
    lockingMode="all" 
    requestUriIgnorePattern=".*\.(ico|png|gif|jpg|css|js)$"
    transcoderFactoryClass="de.javakaffee.web.msm.JavaSerializationTranscoderFactory"/>
</Context>

I have patched LockingStrategy.java for my usage, but the strategy to compute 
memcached keys should be fixed in general. The problem is that the 
storageKeyPrefix is prepended multiple times in many code paths.

Therefore I don't submit my quick and dirty patch to LockingStrategy.java.

Original issue reported on code.google.com by bernard....@seekda.com on 6 Aug 2014 at 11:31

GoogleCodeExporter commented 8 years ago
Thanks for reporting this! Can you point out which code is buggy / should be 
changed?

Original comment by martin.grotzke on 7 Aug 2014 at 11:02

GoogleCodeExporter commented 8 years ago
Method loadSessionValidityInfoForValidityKey is called with e.g. 
"validity:prefix_ASDFE213SRF23". _storageKeyFormat.format() turns it into 
"prefix_validity:prefix_ASDFE213SRF23".

I didn't track down everything. Fact is that loading a session from the backup 
memcached node doesn't work because memcached is queried for "validity:..." 
instead of "prefix_validity:...".

I did the simple fix below, and things started to work for me. However the code 
is not clean with regard to memcached keys used: Meaning the prefix is 
contained multiple times in some keys which is a waste of mem and a source of 
future bugs.

let me congrat here for a great software product to mitigate the offensive tone 
above. I appreciate the concepts and design of msm a lot.

here is my current diff against 1.8.2:

diff --git a/core/src/main/java/de/javakaffee/web/msm/LockingStrategy.java 
b/core/src/main/java/de/javakaffee/web/msm/LockingStrategy.java
index 67668b3..4a891ed 100644
--- a/core/src/main/java/de/javakaffee/web/msm/LockingStrategy.java
+++ b/core/src/main/java/de/javakaffee/web/msm/LockingStrategy.java
@@ -330,7 +330,7 @@ public abstract class LockingStrategy {

     @CheckForNull
     protected SessionValidityInfo loadSessionValidityInfoForValidityKey( @Nonnull final String validityInfoKey ) {
-        final byte[] validityInfo = (byte[]) _memcached.get( 
_storageKeyFormat.format( validityInfoKey ) );
+        final byte[] validityInfo = (byte[]) _memcached.get( validityInfoKey );
         return validityInfo != null ? decode( validityInfo ) : null;
     }

Original comment by bernard....@seekda.com on 8 Aug 2014 at 7:16

GoogleCodeExporter commented 8 years ago
Should be fixed in the attached jar, replaces 
memcached-session-manager-1.8.2.jar. Can you tell if this fixes the issue for 
you?

Original comment by martin.grotzke on 6 Nov 2014 at 11:09

Attachments:

GoogleCodeExporter commented 8 years ago
I just release msm 1.8.3 with this fix.

Original comment by martin.grotzke on 6 Nov 2014 at 10:01