You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by st...@apache.org on 2013/11/15 16:35:05 UTC
svn commit: r1542292 - in /jackrabbit/oak/trunk/oak-mk/src:
main/java/org/apache/jackrabbit/mk/persistence/
main/java/org/apache/jackrabbit/mk/store/
test/java/org/apache/jackrabbit/mk/store/
Author: stefan
Date: Fri Nov 15 15:35:05 2013
New Revision: 1542292
URL: http://svn.apache.org/r1542292
Log:
OAK-587: DefaultRevisionStoreTest.testConcurrentGC fails every now and then.
applied Patch for aggressive GC test
fixed concurrency issue
Modified:
jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/persistence/InMemPersistence.java
jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/store/DefaultRevisionStoreTest.java
Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/persistence/InMemPersistence.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/persistence/InMemPersistence.java?rev=1542292&r1=1542291&r2=1542292&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/persistence/InMemPersistence.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/persistence/InMemPersistence.java Fri Nov 15 15:35:05 2013
@@ -33,6 +33,7 @@ import java.io.File;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
/**
*
@@ -44,6 +45,11 @@ public class InMemPersistence implements
private long gcStart;
+ /**
+ * Read-write lock for objects collection.
+ */
+ private final ReentrantReadWriteLock objectsLock = new ReentrantReadWriteLock();
+
// TODO: make this configurable
private IdFactory idFactory = IdFactory.getDigestFactory();
@@ -61,9 +67,27 @@ public class InMemPersistence implements
}
+ private byte[] get(Id id) {
+ objectsLock.readLock().lock();
+ try {
+ return objects.get(id);
+ } finally {
+ objectsLock.readLock().unlock();
+ }
+ }
+
+ private byte[] put(Id id, byte[] bytes) {
+ objectsLock.writeLock().lock();
+ try {
+ return objects.put(id, bytes);
+ } finally {
+ objectsLock.writeLock().unlock();
+ }
+ }
+
public void readNode(StoredNode node) throws NotFoundException, Exception {
Id id = node.getId();
- byte[] bytes = objects.get(id);
+ byte[] bytes = get(id);
if (bytes != null) {
node.deserialize(new BinaryBinding(new ByteArrayInputStream(bytes)));
return;
@@ -77,9 +101,8 @@ public class InMemPersistence implements
byte[] bytes = out.toByteArray();
Id id = new Id(idFactory.createContentId(bytes));
- if (!objects.containsKey(id)) {
- objects.put(id, bytes);
- }
+ put(id, bytes);
+
if (gcStart != 0) {
marked.put(id, bytes);
}
@@ -87,7 +110,7 @@ public class InMemPersistence implements
}
public StoredCommit readCommit(Id id) throws NotFoundException, Exception {
- byte[] bytes = objects.get(id);
+ byte[] bytes = get(id);
if (bytes != null) {
return StoredCommit.deserialize(id, new BinaryBinding(new ByteArrayInputStream(bytes)));
}
@@ -99,16 +122,15 @@ public class InMemPersistence implements
commit.serialize(new BinaryBinding(out));
byte[] bytes = out.toByteArray();
- if (!objects.containsKey(id)) {
- objects.put(id, bytes);
- }
+ put(id, bytes);
+
if (gcStart != 0) {
marked.put(id, bytes);
}
}
public ChildNodeEntriesMap readCNEMap(Id id) throws NotFoundException, Exception {
- byte[] bytes = objects.get(id);
+ byte[] bytes = get(id);
if (bytes != null) {
return ChildNodeEntriesMap.deserialize(new BinaryBinding(new ByteArrayInputStream(bytes)));
}
@@ -121,9 +143,8 @@ public class InMemPersistence implements
byte[] bytes = out.toByteArray();
Id id = new Id(idFactory.createContentId(bytes));
- if (!objects.containsKey(id)) {
- objects.put(id, bytes);
- }
+ put(id, bytes);
+
if (gcStart != 0) {
marked.put(id, bytes);
}
@@ -162,12 +183,12 @@ public class InMemPersistence implements
commit.serialize(new BinaryBinding(out));
byte[] bytes = out.toByteArray();
- objects.put(id, bytes);
+ put(id, bytes);
marked.put(id, bytes);
}
private boolean markObject(Id id) throws NotFoundException {
- byte[] data = objects.get(id);
+ byte[] data = get(id);
if (data != null) {
return marked.put(id, data) == null;
}
@@ -176,12 +197,19 @@ public class InMemPersistence implements
@Override
public int sweep() {
+ objectsLock.writeLock().lock();
+
int count = objects.size();
-
- objects.clear();
- objects.putAll(marked);
-
- gcStart = 0;
- return count;
+
+ try {
+ objects.clear();
+ objects.putAll(marked);
+
+ gcStart = 0;
+
+ return count - objects.size();
+ } finally {
+ objectsLock.writeLock().unlock();
+ }
}
}
Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java?rev=1542292&r1=1542291&r2=1542292&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/store/DefaultRevisionStore.java Fri Nov 15 15:35:05 2013
@@ -267,7 +267,7 @@ public class DefaultRevisionStore implem
* Make sure that a GC cycle can not sweep this newly persisted node
* before we have updated our token
*/
- tokensLock.readLock().lock();
+ tokensLock.writeLock().lock();
try {
Id id = pm.writeNode(node);
@@ -285,7 +285,7 @@ public class DefaultRevisionStore implem
return id;
} finally {
- tokensLock.readLock().unlock();
+ tokensLock.writeLock().unlock();
}
}
@@ -329,8 +329,15 @@ public class DefaultRevisionStore implem
Id id = writeCommit(token, commit);
setHeadCommitId(id);
-
- putTokens.remove(token);
+
+ tokensLock.writeLock().lock();
+
+ try {
+ putTokens.remove(token);
+ } finally {
+ tokensLock.writeLock().unlock();
+ }
+
if (branchRevId != null) {
synchronized (branches) {
branches.remove(branchRevId);
@@ -343,7 +350,14 @@ public class DefaultRevisionStore implem
verifyInitialized();
Id commitId = writeCommit(token, commit);
- putTokens.remove(token);
+
+ tokensLock.writeLock().lock();
+
+ try {
+ putTokens.remove(token);
+ } finally {
+ tokensLock.writeLock().unlock();
+ }
Id branchRootId = commit.getBranchRootId();
if (branchRootId != null) {
@@ -531,7 +545,7 @@ public class DefaultRevisionStore implem
* if an error occurs
*/
private void markUncommittedNodes() throws Exception {
- tokensLock.writeLock().lock();
+ tokensLock.readLock().lock();
try {
gcpm.start();
@@ -542,7 +556,7 @@ public class DefaultRevisionStore implem
markNode(token.getLastModified());
}
} finally {
- tokensLock.writeLock().unlock();
+ tokensLock.readLock().unlock();
}
}
Modified: jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/store/DefaultRevisionStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/store/DefaultRevisionStoreTest.java?rev=1542292&r1=1542291&r2=1542292&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/store/DefaultRevisionStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/test/java/org/apache/jackrabbit/mk/store/DefaultRevisionStoreTest.java Fri Nov 15 15:35:05 2013
@@ -40,7 +40,6 @@ import org.json.simple.JSONArray;
import org.json.simple.parser.JSONParser;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
@@ -134,7 +133,6 @@ public class DefaultRevisionStoreTest {
*
* @throws Exception if an error occurs
*/
- @Ignore
@Test
public void testConcurrentGC() throws Exception {
ScheduledExecutorService gcExecutor = Executors.newScheduledThreadPool(1);
@@ -143,17 +141,17 @@ public class DefaultRevisionStoreTest {
public void run() {
rs.gc();
}
- }, 100, 20, TimeUnit.MILLISECONDS);
+ }, 10, 2, TimeUnit.MILLISECONDS);
mk.commit("/", "+\"a\" : { \"b\" : { \"c\" : { \"d\" : {} } } }",
mk.getHeadRevision(), null);
try {
- for (int i = 0; i < 20; i++) {
+ for (int i = 0; i < 2000; i++) {
mk.commit("/a/b/c/d", "+\"e\" : {}", mk.getHeadRevision(), null);
- Thread.sleep(10);
+ Thread.sleep(1);
mk.commit("/a/b/c/d/e", "+\"f\" : {}", mk.getHeadRevision(), null);
- Thread.sleep(30);
+ Thread.sleep(3);
mk.commit("/a/b/c/d", "-\"e\"", mk.getHeadRevision(), null);
}
} finally {
@@ -167,7 +165,6 @@ public class DefaultRevisionStoreTest {
* @throws Exception if an error occurs
*/
@Test
- @Ignore
public void testConcurrentMergeGC() throws Exception {
ScheduledExecutorService gcExecutor = Executors.newScheduledThreadPool(1);
gcExecutor.scheduleWithFixedDelay(new Runnable() {