[jbosscache-commits] JBoss Cache SVN: r7911 - in core/branches/flat/src: main/java/org/horizon/factories and 2 other directories.

jbosscache-commits at lists.jboss.org jbosscache-commits at lists.jboss.org
Wed Mar 18 10:45:40 EDT 2009


Author: manik.surtani at jboss.com
Date: 2009-03-18 10:45:39 -0400 (Wed, 18 Mar 2009)
New Revision: 7911

Added:
   core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntryForRemoval.java
Modified:
   core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntry.java
   core/branches/flat/src/main/java/org/horizon/factories/EntryFactory.java
   core/branches/flat/src/main/java/org/horizon/factories/EntryFactoryImpl.java
   core/branches/flat/src/main/java/org/horizon/interceptors/CacheLoaderInterceptor.java
   core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterceptor.java
   core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/RepeatableReadLockTest.java
Log:
Fixed bug in repeatable read when removing a null entry

Modified: core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntry.java
===================================================================
--- core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntry.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntry.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -28,31 +28,16 @@
  * @author Manik Surtani (<a href="mailto:manik at jboss.org">manik at jboss.org</a>)
  * @since 1.0
  */
-public class NullMarkerEntry extends ReadCommittedEntry {
-   /**
-    * @return always returns true
-    */
-   @Override
-   public boolean isNullEntry() {
-      return true;
-   }
+public class NullMarkerEntry extends NullMarkerEntryForRemoval {
 
-   /**
-    * @return always returns true so that any get commands, upon getting this entry, will ignore the entry as though it
-    *         were removed.
-    */
-   @Override
-   public boolean isDeleted() {
-      return true;
+   private static final NullMarkerEntry INSTANCE = new NullMarkerEntry();
+
+   private NullMarkerEntry() {
+      super(null);
    }
 
-   /**
-    * @return always returns true so that any get commands, upon getting this entry, will ignore the entry as though it
-    *         were invalid.
-    */
-   @Override
-   public boolean isValid() {
-      return false;
+   public static NullMarkerEntry getInstance() {
+      return INSTANCE;
    }
 
    /**

Added: core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntryForRemoval.java
===================================================================
--- core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntryForRemoval.java	                        (rev 0)
+++ core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntryForRemoval.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -0,0 +1,41 @@
+package org.horizon.container;
+
+/**
+ * A null entry that is read in for removal
+ *
+ * @author Manik Surtani
+ * @since 1.0
+ */
+public class NullMarkerEntryForRemoval extends RepeatableReadEntry {
+
+   public NullMarkerEntryForRemoval(Object key) {
+      super(key, null, -1);
+   }
+
+   /**
+    * @return always returns true
+    */
+   @Override
+   public boolean isNullEntry() {
+      return true;
+   }
+
+   /**
+    * @return always returns true so that any get commands, upon getting this entry, will ignore the entry as though it
+    *         were removed.
+    */
+   @Override
+   public boolean isDeleted() {
+      return true;
+   }
+
+   /**
+    * @return always returns true so that any get commands, upon getting this entry, will ignore the entry as though it
+    *         were invalid.
+    */
+   @Override
+   public boolean isValid() {
+      return false;
+   }
+
+}

Modified: core/branches/flat/src/main/java/org/horizon/factories/EntryFactory.java
===================================================================
--- core/branches/flat/src/main/java/org/horizon/factories/EntryFactory.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/factories/EntryFactory.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -48,7 +48,7 @@
     */
    boolean acquireLock(InvocationContext ctx, Object key) throws InterruptedException, TimeoutException;
 
-   MVCCEntry wrapEntryForWriting(InvocationContext ctx, Object key, boolean createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLocked) throws InterruptedException;
+   MVCCEntry wrapEntryForWriting(InvocationContext ctx, Object key, boolean createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLocked, boolean forRemoval) throws InterruptedException;
 
    MVCCEntry wrapEntryForReading(InvocationContext ctx, Object key) throws InterruptedException;
 }

Modified: core/branches/flat/src/main/java/org/horizon/factories/EntryFactoryImpl.java
===================================================================
--- core/branches/flat/src/main/java/org/horizon/factories/EntryFactoryImpl.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/factories/EntryFactoryImpl.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -26,6 +26,7 @@
 import org.horizon.container.DataContainer;
 import org.horizon.container.MVCCEntry;
 import org.horizon.container.NullMarkerEntry;
+import org.horizon.container.NullMarkerEntryForRemoval;
 import org.horizon.container.ReadCommittedEntry;
 import org.horizon.container.RepeatableReadEntry;
 import org.horizon.container.UpdateableEntry;
@@ -42,7 +43,6 @@
 
 public class EntryFactoryImpl implements EntryFactory {
    private boolean useRepeatableRead;
-   private static final NullMarkerEntry NULL_MARKER = new NullMarkerEntry();
    DataContainer container;
    boolean writeSkewCheck;
    LockManager lockManager;
@@ -66,8 +66,10 @@
       writeSkewCheck = configuration.isWriteSkewCheck();
    }
 
-   public final UpdateableEntry createWrappedEntry(Object key, Object value, boolean isForInsert, long lifespan) {
-      if (value == null && !isForInsert) return useRepeatableRead ? NULL_MARKER : null;
+   private UpdateableEntry createWrappedEntry(Object key, Object value, boolean isForInsert, boolean forRemoval, long lifespan) {
+      if (value == null && !isForInsert) return useRepeatableRead ?
+            forRemoval ? new NullMarkerEntryForRemoval(key) : NullMarkerEntry.getInstance()
+            : null;
 
       return useRepeatableRead ? new RepeatableReadEntry(key, value, lifespan) : new ReadCommittedEntry(key, value, lifespan);
    }
@@ -76,7 +78,7 @@
       MVCCEntry mvccEntry;
       if (ctx.hasOption(Options.FORCE_WRITE_LOCK)) {
          if (trace) log.trace("Forcing lock on reading");
-         return wrapEntryForWriting(ctx, key, false, false, false);
+         return wrapEntryForWriting(ctx, key, false, false, false, false);
       } else if ((mvccEntry = ctx.lookupEntry(key)) == null) {
          if (trace) log.trace("Key " + key + " is not in context, fetching from container.");
          // simple implementation.  Peek the entry, wrap it, put wrapped entry in the context.
@@ -88,8 +90,8 @@
             if (se != null) ctx.putLookedUpEntry(key, se);
          } else {
             mvccEntry = se == null ?
-                  createWrappedEntry(key, null, false, -1) :
-                  createWrappedEntry(key, se.getValue(), false, se.getLifespan());
+                  createWrappedEntry(key, null, false, false, -1) :
+                  createWrappedEntry(key, se.getValue(), false, false, se.getLifespan());
             if (mvccEntry != null) ctx.putLookedUpEntry(key, mvccEntry);
          }
 
@@ -100,7 +102,7 @@
       }
    }
 
-   public final MVCCEntry wrapEntryForWriting(InvocationContext ctx, Object key, boolean createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLocked) throws InterruptedException {
+   public final MVCCEntry wrapEntryForWriting(InvocationContext ctx, Object key, boolean createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLocked, boolean forRemoval) throws InterruptedException {
       MVCCEntry mvccEntry = ctx.lookupEntry(key);
       if (createIfAbsent && mvccEntry != null && mvccEntry.isNullEntry()) mvccEntry = null;
       if (mvccEntry != null) // exists in context!  Just acquire lock if needed, and wrap.
@@ -108,11 +110,12 @@
          // acquire lock if needed
          if (alreadyLocked || acquireLock(ctx, key)) {
             UpdateableEntry ue;
-            if (mvccEntry instanceof UpdateableEntry) {
+
+            if (mvccEntry instanceof UpdateableEntry && (!forRemoval || !(mvccEntry instanceof NullMarkerEntry))) {
                ue = (UpdateableEntry) mvccEntry;
             } else {
                // this is a read-only entry that needs to be copied to a proper read-write entry!!
-               ue = createWrappedEntry(key, mvccEntry.getValue(), false, mvccEntry.getLifespan());
+               ue = createWrappedEntry(key, mvccEntry.getValue(), false, forRemoval, mvccEntry.getLifespan());
                mvccEntry = ue;
                ctx.putLookedUpEntry(key, mvccEntry);
             }
@@ -134,7 +137,7 @@
             // exists in cache!  Just acquire lock if needed, and wrap.
             // do we need a lock?
             boolean needToCopy = alreadyLocked || acquireLock(ctx, key) || ctx.hasOption(Options.SKIP_LOCKING); // even if we do not acquire a lock, if skip-locking is enabled we should copy
-            UpdateableEntry ue = createWrappedEntry(key, cachedValue.getValue(), false, cachedValue.getLifespan());
+            UpdateableEntry ue = createWrappedEntry(key, cachedValue.getValue(), false, false, cachedValue.getLifespan());
             ctx.putLookedUpEntry(key, ue);
             if (needToCopy) ue.copyForUpdate(container, writeSkewCheck);
             mvccEntry = ue;
@@ -144,7 +147,7 @@
             // now to lock and create the entry.  Lock first to prevent concurrent creation!
             if (!alreadyLocked) acquireLock(ctx, key);
             notifier.notifyCacheEntryCreated(key, true, ctx);
-            UpdateableEntry ue = createWrappedEntry(key, null, true, -1);
+            UpdateableEntry ue = createWrappedEntry(key, null, true, false, -1);
             ue.setCreated(true);
             ctx.putLookedUpEntry(key, ue);
             ue.copyForUpdate(container, writeSkewCheck);

Modified: core/branches/flat/src/main/java/org/horizon/interceptors/CacheLoaderInterceptor.java
===================================================================
--- core/branches/flat/src/main/java/org/horizon/interceptors/CacheLoaderInterceptor.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/interceptors/CacheLoaderInterceptor.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -126,7 +126,7 @@
          }
 
          // Reuse the lock and create a new entry for loading
-         MVCCEntry n = entryFactory.wrapEntryForWriting(ctx, key, true, false, keyLocked);
+         MVCCEntry n = entryFactory.wrapEntryForWriting(ctx, key, true, false, keyLocked, false);
          n = loadEntry(ctx, key, n);
          ctx.setContainsModifications(true);
       }

Modified: core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterceptor.java
===================================================================
--- core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterceptor.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterceptor.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -129,7 +129,8 @@
    public Object visitClearCommand(InvocationContext ctx, ClearCommand command) throws Throwable {
       try {
          // get a snapshot of all keys in the data container
-         for (Object key : dataContainer.keySet()) entryFactory.wrapEntryForWriting(ctx, key, false, false, false);
+         for (Object key : dataContainer.keySet())
+            entryFactory.wrapEntryForWriting(ctx, key, false, false, false, false);
          ctx.setContainsModifications(true);
          return invokeNextInterceptor(ctx, command);
       } finally {
@@ -147,7 +148,7 @@
    public Object visitInvalidateCommand(InvocationContext ctx, InvalidateCommand command) throws Throwable {
       try {
          if (command.getKeys() != null) {
-            for (Object key : command.getKeys()) entryFactory.wrapEntryForWriting(ctx, key, false, true, false);
+            for (Object key : command.getKeys()) entryFactory.wrapEntryForWriting(ctx, key, false, true, false, false);
          }
          Object o = invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(command.isSuccessful());
@@ -160,7 +161,7 @@
    @Override
    public Object visitPutKeyValueCommand(InvocationContext ctx, PutKeyValueCommand command) throws Throwable {
       try {
-         entryFactory.wrapEntryForWriting(ctx, command.getKey(), true, false, false);
+         entryFactory.wrapEntryForWriting(ctx, command.getKey(), true, false, false, false);
          Object o = invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(command.isSuccessful());
          return o;
@@ -173,7 +174,7 @@
    public Object visitPutMapCommand(InvocationContext ctx, PutMapCommand command) throws Throwable {
       try {
          for (Object key : command.getMap().keySet()) {
-            entryFactory.wrapEntryForWriting(ctx, key, true, false, false);
+            entryFactory.wrapEntryForWriting(ctx, key, true, false, false, false);
          }
          Object o = invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(command.isSuccessful());
@@ -187,7 +188,7 @@
    @Override
    public Object visitRemoveCommand(InvocationContext ctx, RemoveCommand command) throws Throwable {
       try {
-         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, true, false);
+         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, true, false, true);
          Object o = invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(command.isSuccessful());
          return o;
@@ -200,7 +201,7 @@
    @Override
    public Object visitReplaceCommand(InvocationContext ctx, ReplaceCommand command) throws Throwable {
       try {
-         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, true, false);
+         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, true, false, false);
          Object o = invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(command.isSuccessful());
          return o;

Modified: core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/RepeatableReadLockTest.java
===================================================================
--- core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/RepeatableReadLockTest.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/RepeatableReadLockTest.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -95,4 +95,21 @@
       assert "v".equals(cache.get("k"));
       assertNoLocks();
    }
+
+   public void testRepeatableReadWithNullRemoval() throws Exception {
+      LockTestBaseTL tl = threadLocal.get();
+      Cache<String, String> cache = tl.cache;
+      TransactionManager tm = tl.tm;
+
+      // start with an empty cache
+      tm.begin();
+      cache.get("a");
+      Transaction tx = tm.suspend();
+      cache.put("a", "v2");
+      assert cache.get("a").equals("v2");
+      tm.resume(tx);
+      cache.remove("a");
+      tx.commit();
+      assert cache.get("a") == null;
+   }
 }




More information about the jbosscache-commits mailing list