3:45 p.m.
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/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@jboss.org">manik@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;
+ }
}