From jbosscache-commits at lists.jboss.org Wed Mar 18 10:45:40 2009
Content-Type: multipart/mixed; boundary="===============6173594154594331994=="
MIME-Version: 1.0
From: jbosscache-commits at lists.jboss.org
To: jbosscache-commits at lists.jboss.org
Subject: [jbosscache-commits] JBoss Cache SVN: r7911 - in
 core/branches/flat/src: main/java/org/horizon/factories and 2 other
 directories.
Date: Wed, 18 Mar 2009 10:45:40 -0400
Message-ID: <E1Ljx1Y-0002aY-Ex@committer01.frg.pub.inap.atl.jboss.com>

--===============6173594154594331994==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

Author: manik.surtani(a)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/NullMarkerEntryFo=
rRemoval.java
Modified:
   core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntry.j=
ava
   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/CacheLoaderInt=
erceptor.java
   core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterce=
ptor.java
   core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/Re=
peatableReadLockTest.java
Log:
Fixed bug in repeatable read when removing a null entry

Modified: core/branches/flat/src/main/java/org/horizon/container/NullMarker=
Entry.java
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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=3D"mailto:manik(a)jboss.org">manik(a)jbo=
ss.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 t=
his entry, will ignore the entry as though it
-    *         were removed.
-    */
-   @Override
-   public boolean isDeleted() {
-      return true;
+   private static final NullMarkerEntry INSTANCE =3D new NullMarkerEntry();
+
+   private NullMarkerEntry() {
+      super(null);
    }
 =

-   /**
-    * @return always returns true so that any get commands, upon getting t=
his 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/NullMarkerEnt=
ryForRemoval.java
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntryF=
orRemoval.java	                        (rev 0)
+++ core/branches/flat/src/main/java/org/horizon/container/NullMarkerEntryF=
orRemoval.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 t=
his 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 t=
his 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/EntryFacto=
ry.java
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- core/branches/flat/src/main/java/org/horizon/factories/EntryFactory.jav=
a	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/factories/EntryFactory.jav=
a	2009-03-18 14:45:39 UTC (rev 7911)
@@ -48,7 +48,7 @@
     */
    boolean acquireLock(InvocationContext ctx, Object key) throws Interrupt=
edException, TimeoutException;
 =

-   MVCCEntry wrapEntryForWriting(InvocationContext ctx, Object key, boolea=
n createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLocked) throws =
InterruptedException;
+   MVCCEntry wrapEntryForWriting(InvocationContext ctx, Object key, boolea=
n 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/EntryFacto=
ryImpl.java
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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 =3D new NullMarkerEntr=
y();
    DataContainer container;
    boolean writeSkewCheck;
    LockManager lockManager;
@@ -66,8 +66,10 @@
       writeSkewCheck =3D configuration.isWriteSkewCheck();
    }
 =

-   public final UpdateableEntry createWrappedEntry(Object key, Object valu=
e, boolean isForInsert, long lifespan) {
-      if (value =3D=3D null && !isForInsert) return useRepeatableRead ? NU=
LL_MARKER : null;
+   private UpdateableEntry createWrappedEntry(Object key, Object value, bo=
olean isForInsert, boolean forRemoval, long lifespan) {
+      if (value =3D=3D null && !isForInsert) return useRepeatableRead ?
+            forRemoval ? new NullMarkerEntryForRemoval(key) : NullMarkerEn=
try.getInstance()
+            : null;
 =

       return useRepeatableRead ? new RepeatableReadEntry(key, value, lifes=
pan) : 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 =3D ctx.lookupEntry(key)) =3D=3D null) {
          if (trace) log.trace("Key " + key + " is not in context, fetching=
 from container.");
          // simple implementation.  Peek the entry, wrap it, put wrapped e=
ntry in the context.
@@ -88,8 +90,8 @@
             if (se !=3D null) ctx.putLookedUpEntry(key, se);
          } else {
             mvccEntry =3D se =3D=3D null ?
-                  createWrappedEntry(key, null, false, -1) :
-                  createWrappedEntry(key, se.getValue(), false, se.getLife=
span());
+                  createWrappedEntry(key, null, false, false, -1) :
+                  createWrappedEntry(key, se.getValue(), false, false, se.=
getLifespan());
             if (mvccEntry !=3D null) ctx.putLookedUpEntry(key, mvccEntry);
          }
 =

@@ -100,7 +102,7 @@
       }
    }
 =

-   public final MVCCEntry wrapEntryForWriting(InvocationContext ctx, Objec=
t key, boolean createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLo=
cked) throws InterruptedException {
+   public final MVCCEntry wrapEntryForWriting(InvocationContext ctx, Objec=
t key, boolean createIfAbsent, boolean forceLockIfAbsent, boolean alreadyLo=
cked, boolean forRemoval) throws InterruptedException {
       MVCCEntry mvccEntry =3D ctx.lookupEntry(key);
       if (createIfAbsent && mvccEntry !=3D null && mvccEntry.isNullEntry()=
) mvccEntry =3D null;
       if (mvccEntry !=3D 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 =3D (UpdateableEntry) mvccEntry;
             } else {
                // this is a read-only entry that needs to be copied to a p=
roper read-write entry!!
-               ue =3D createWrappedEntry(key, mvccEntry.getValue(), false,=
 mvccEntry.getLifespan());
+               ue =3D createWrappedEntry(key, mvccEntry.getValue(), false,=
 forRemoval, mvccEntry.getLifespan());
                mvccEntry =3D 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 =3D 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 =3D createWrappedEntry(key, cachedValue.get=
Value(), false, cachedValue.getLifespan());
+            UpdateableEntry ue =3D createWrappedEntry(key, cachedValue.get=
Value(), false, false, cachedValue.getLifespan());
             ctx.putLookedUpEntry(key, ue);
             if (needToCopy) ue.copyForUpdate(container, writeSkewCheck);
             mvccEntry =3D ue;
@@ -144,7 +147,7 @@
             // now to lock and create the entry.  Lock first to prevent co=
ncurrent creation!
             if (!alreadyLocked) acquireLock(ctx, key);
             notifier.notifyCacheEntryCreated(key, true, ctx);
-            UpdateableEntry ue =3D createWrappedEntry(key, null, true, -1);
+            UpdateableEntry ue =3D createWrappedEntry(key, null, true, fal=
se, -1);
             ue.setCreated(true);
             ctx.putLookedUpEntry(key, ue);
             ue.copyForUpdate(container, writeSkewCheck);

Modified: core/branches/flat/src/main/java/org/horizon/interceptors/CacheLo=
aderInterceptor.java
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- core/branches/flat/src/main/java/org/horizon/interceptors/CacheLoaderIn=
terceptor.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/interceptors/CacheLoaderIn=
terceptor.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 =3D entryFactory.wrapEntryForWriting(ctx, key, true, =
false, keyLocked);
+         MVCCEntry n =3D entryFactory.wrapEntryForWriting(ctx, key, true, =
false, keyLocked, false);
          n =3D loadEntry(ctx, key, n);
          ctx.setContainsModifications(true);
       }

Modified: core/branches/flat/src/main/java/org/horizon/interceptors/Locking=
Interceptor.java
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterc=
eptor.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/main/java/org/horizon/interceptors/LockingInterc=
eptor.java	2009-03-18 14:45:39 UTC (rev 7911)
@@ -129,7 +129,8 @@
    public Object visitClearCommand(InvocationContext ctx, ClearCommand com=
mand) throws Throwable {
       try {
          // get a snapshot of all keys in the data container
-         for (Object key : dataContainer.keySet()) entryFactory.wrapEntryF=
orWriting(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, InvalidateC=
ommand command) throws Throwable {
       try {
          if (command.getKeys() !=3D null) {
-            for (Object key : command.getKeys()) entryFactory.wrapEntryFor=
Writing(ctx, key, false, true, false);
+            for (Object key : command.getKeys()) entryFactory.wrapEntryFor=
Writing(ctx, key, false, true, false, false);
          }
          Object o =3D invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(=
command.isSuccessful());
@@ -160,7 +161,7 @@
    @Override
    public Object visitPutKeyValueCommand(InvocationContext ctx, PutKeyValu=
eCommand command) throws Throwable {
       try {
-         entryFactory.wrapEntryForWriting(ctx, command.getKey(), true, fal=
se, false);
+         entryFactory.wrapEntryForWriting(ctx, command.getKey(), true, fal=
se, false, false);
          Object o =3D invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(=
command.isSuccessful());
          return o;
@@ -173,7 +174,7 @@
    public Object visitPutMapCommand(InvocationContext ctx, PutMapCommand c=
ommand) 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 =3D invokeNextInterceptor(ctx, command);
          if (!ctx.isContainsModifications()) ctx.setContainsModifications(=
command.isSuccessful());
@@ -187,7 +188,7 @@
    @Override
    public Object visitRemoveCommand(InvocationContext ctx, RemoveCommand c=
ommand) throws Throwable {
       try {
-         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, tr=
ue, false);
+         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, tr=
ue, false, true);
          Object o =3D 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, tr=
ue, false);
+         entryFactory.wrapEntryForWriting(ctx, command.getKey(), false, tr=
ue, false, false);
          Object o =3D 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/R=
epeatableReadLockTest.java	2009-03-18 14:02:03 UTC (rev 7910)
+++ core/branches/flat/src/test/java/org/horizon/api/mvcc/repeatable_read/R=
epeatableReadLockTest.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 =3D threadLocal.get();
+      Cache<String, String> cache =3D tl.cache;
+      TransactionManager tm =3D tl.tm;
+
+      // start with an empty cache
+      tm.begin();
+      cache.get("a");
+      Transaction tx =3D tm.suspend();
+      cache.put("a", "v2");
+      assert cache.get("a").equals("v2");
+      tm.resume(tx);
+      cache.remove("a");
+      tx.commit();
+      assert cache.get("a") =3D=3D null;
+   }
 }


--===============6173594154594331994==--