Author: mreutegg Date: Mon Jun 16 06:58:35 2014 New Revision: 1602810 URL: http://svn.apache.org/r1602810 Log: OAK-1788: ConcurrentConflictTest fails occasionally
Merge r1602179 from trunk Modified: jackrabbit/oak/branches/1.0/ (props changed) jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentConflictTest.java jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/RevisionTest.java Propchange: jackrabbit/oak/branches/1.0/ ------------------------------------------------------------------------------ Merged /jackrabbit/oak/trunk:r1602179 Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java?rev=1602810&r1=1602809&r2=1602810&view=diff ============================================================================== --- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java (original) +++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Revision.java Mon Jun 16 06:58:35 2014 @@ -549,6 +549,9 @@ public class Revision { * <ul> * <li> * {@code null} if the revision is older than the earliest range + * and the revision timestamp is less than or equal the time + * of the last {@link #purge(long)} (see also + * {@link #oldestTimestamp}). * </li> * <li> * if the revision is newer than the lower bound of the newest @@ -565,9 +568,49 @@ public class Revision { * </li> * </ul> * + * Below is a graph for a revision comparison example as seen from one + * cluster node with some known revision ranges. Revision ranges less + * than or equal r2-0-0 have been purged and there are known ranges for + * cluster node 1 (this cluster node) and cluster node 2 (some other + * cluster node). + * <pre> + * View from cluster node 1: + * + * purge r3-0-1 r5-0-2 r7-0-1 + * Ë Ë Ë Ë + * ---+---------+---------+---------+---------+--------- + * r1-0-0 r2-0-0 r3-0-0 r4-0-0 r5-0-0 + * + * ^ + * r1-0-1 -> null (1) + * + * ^ + * r4-0-2 -> r4-0-0 (2) + * + * ^ + * r3-0-1 -> r3-0-0 (3) + * + * ^ + * r6-0-2 -> FUTURE (4) + * + * ^ + * r9-0-1 -> NEWEST (5) + * </pre> + * <ol> + * <li>older than earliest range and purge time</li> + * <li>seen-at of next higher range</li> + * <li>seen-at of matching lower bound of range</li> + * <li>foreign revision is newer than most recent range</li> + * <li>local revision is newer than most recent range</li> + * </ol> + * This gives the following revision ordering: + * <pre> + * r1-0-1 < r3-0-1 < r-4-0-2 < r9-0-1 < r6-0-2 + * </pre> + * * @param r the revision * @return the seen-at revision or {@code null} if the revision is older - * than the earliest range. + * than the earliest range and purge time. */ Revision getRevisionSeen(Revision r) { List<RevisionRange> list = map.get(r.getClusterId()); @@ -586,8 +629,9 @@ public class Revision { // search from latest backward // (binary search could be used, but we expect most queries // at the end of the list) + RevisionRange range = null; for (int i = list.size() - 1; i >= 0; i--) { - RevisionRange range = list.get(i); + range = list.get(i); int compare = r.compareRevisionTime(range.revision); if (compare == 0) { return range.seenAt; @@ -597,15 +641,21 @@ public class Revision { if (r.getClusterId() == currentClusterNodeId) { // newer than all others, except for FUTURE return NEWEST; + } else { + // happens in the future (not visible yet) + return FUTURE; } - // happens in the future (not visible yet) - return FUTURE; } else { // there is a newer range return list.get(i + 1).seenAt; } } } + if (range != null && r.getTimestamp() > oldestTimestamp) { + // revision is older than earliest range and after purge + // timestamp. return seen-at revision of earliest range. + return range.seenAt; + } return null; } Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentConflictTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentConflictTest.java?rev=1602810&r1=1602809&r2=1602810&view=diff ============================================================================== --- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentConflictTest.java (original) +++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentConflictTest.java Mon Jun 16 06:58:35 2014 @@ -30,6 +30,7 @@ import org.apache.jackrabbit.mk.api.Micr import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore; import org.json.simple.JSONObject; import org.json.simple.parser.JSONParser; +import org.junit.After; import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -68,6 +69,16 @@ public class ConcurrentConflictTest exte } } + @After + @Override + public void disposeDocumentMK() { + super.disposeDocumentMK(); + for (DocumentMK mk : kernels) { + mk.dispose(); + } + kernels.clear(); + } + private DocumentMK openDocumentMK() { return new DocumentMK.Builder().setAsyncDelay(10).setDocumentStore(store).open(); } @@ -78,12 +89,23 @@ public class ConcurrentConflictTest exte concurrentUpdates(true); } - @Ignore("OAK-1788") @Test public void concurrentUpdates() throws Exception { concurrentUpdates(false); } + @Ignore("Enable to run concurrentUpdates() in a loop") + @Test + public void concurrentUpdates_Loop() throws Exception { + for (int i = 0; i < 1000; i++) { + System.out.println("test " + i); + concurrentUpdates(false); + // prepare for next round + disposeDocumentMK(); + initDocumentMK(); + } + } + private void concurrentUpdates(final boolean useBranch) throws Exception { LOG.info("====== Start test ======="); final AtomicInteger conflicts = new AtomicInteger(); @@ -97,9 +119,9 @@ public class ConcurrentConflictTest exte @Override public void run() { BitSet conflictSet = new BitSet(); - int numTransfers = NUM_TRANSFERS_PER_THREAD; + int numTransfers = 0; try { - while (numTransfers > 0) { + while (numTransfers < NUM_TRANSFERS_PER_THREAD && exceptions.isEmpty()) { try { if (!transfer()) { continue; @@ -110,7 +132,7 @@ public class ConcurrentConflictTest exte conflicts.incrementAndGet(); conflictSet.set(numTransfers); } - numTransfers--; + numTransfers++; } } catch (Exception e) { exceptions.add(e); @@ -161,6 +183,10 @@ public class ConcurrentConflictTest exte rev = mk.merge(rev, null); } log("Successful transfer @" + oldRev + ": " + jsop.toString() + " (new rev: " + rev + ")"); + long s = calculateSum(mk, rev); + if (s != NUM_NODES * 100) { + throw new Exception("Sum mismatch: " + s); + } return true; } })); @@ -177,13 +203,7 @@ public class ConcurrentConflictTest exte } DocumentMK mk = openDocumentMK(); String rev = mk.getHeadRevision(); - long sum = 0; - for (int i = 0; i < NUM_NODES; i++) { - String json = mk.getNodes("/node-" + i, rev, 0, 0, 1000, null); - JSONParser parser = new JSONParser(); - JSONObject obj = (JSONObject) parser.parse(json); - sum += (Long) obj.get("value"); - } + long sum = calculateSum(mk, rev); log("Conflict rate: " + conflicts.get() + "/" + (NUM_WRITERS * NUM_TRANSFERS_PER_THREAD)); System.out.print(logBuffer); @@ -191,6 +211,18 @@ public class ConcurrentConflictTest exte if (!exceptions.isEmpty()) { throw exceptions.get(0); } + mk.dispose(); + } + + static long calculateSum(MicroKernel mk, String rev) throws Exception { + long sum = 0; + for (int i = 0; i < NUM_NODES; i++) { + String json = mk.getNodes("/node-" + i, rev, 0, 0, 1000, null); + JSONParser parser = new JSONParser(); + JSONObject obj = (JSONObject) parser.parse(json); + sum += (Long) obj.get("value"); + } + return sum; } void log(String msg) { Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/RevisionTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/RevisionTest.java?rev=1602810&r1=1602809&r2=1602810&view=diff ============================================================================== --- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/RevisionTest.java (original) +++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/RevisionTest.java Mon Jun 16 06:58:35 2014 @@ -164,7 +164,8 @@ public class RevisionTest { "1:\n r120-0-1:r20-0-0\n" + "2:\n r200-0-2:r10-0-0\n", comp.toString()); - assertEquals(-1, comp.compare(r0c1, r0c2)); + // r0c2 happens before r0c1 because r2c2 is declared before r2c1 + assertEquals(1, comp.compare(r0c1, r0c2)); assertEquals(1, comp.compare(r1c1, r1c2)); assertEquals(1, comp.compare(r2c1, r2c2)); @@ -221,17 +222,18 @@ public class RevisionTest { Revision r2c2 = new Revision(0x40, 0, 2); comp.add(r1c1, new Revision(0x10, 0, 0)); - comp.add(r2c1, new Revision(0x20, 0, 0)); + comp.add(r2c1, new Revision(0x30, 0, 0)); // there's no range for c2, and therefore this // revision must be considered to be in the future assertTrue(comp.compare(r1c2, r2c1) > 0); // add a range for r2r2 - comp.add(r2c2, new Revision(0x30, 0, 0)); + comp.add(r2c2, new Revision(0x40, 0, 0)); + comp.purge(0x20); - // now there is a range for c2, but the revision is old, - // so it must be considered to be in the past + // now there is a range for c2, but the revision is old (before purge + // time, so it must be considered to be in the past assertTrue(comp.compare(r1c2, r2c1) < 0); } @@ -251,7 +253,7 @@ public class RevisionTest { Revision c2sync = Revision.fromString("r4-1-2"); comp.add(c2sync, Revision.fromString("r2-1-0")); Revision c3sync = Revision.fromString("r2-0-3"); - comp.add(c3sync, Revision.fromString("r2-1-0")); + comp.add(c3sync, Revision.fromString("r2-1-0")); assertTrue(comp.compare(r1, r2) < 0); assertTrue(comp.compare(r2, c2sync) < 0); @@ -267,6 +269,7 @@ public class RevisionTest { @Test public void revisionSeen() { RevisionComparator comp = new RevisionComparator(1); + comp.purge(0); Revision r0 = new Revision(0x01, 0, 1); Revision r1 = new Revision(0x10, 0, 1); @@ -281,8 +284,9 @@ public class RevisionTest { comp.add(r3, new Revision(0x30, 0, 0)); comp.add(r4, new Revision(0x40, 0, 0)); - // older than first range -> must return null - assertNull(comp.getRevisionSeen(r0)); + // older than first range, but after purge timestamp + // -> must return seen-at of first range + assertEquals(new Revision(0x10, 0, 0), comp.getRevisionSeen(r0)); // exact range start matches assertEquals(new Revision(0x10, 0, 0), comp.getRevisionSeen(r1)); @@ -323,6 +327,29 @@ public class RevisionTest { assertNull(comp.getRevisionSeen(r2)); } + // OAK-1822 + @Test + public void seenAtBeforeFirstRangeAfterPurge() { + RevisionComparator comp = new RevisionComparator(1); + comp.purge(0); + + Revision r1 = new Revision(1, 0, 1); + Revision r2 = new Revision(2, 0, 1); + Revision r3 = new Revision(3, 0, 1); + + Revision r3seen = new Revision(3, 0, 0); + + comp.add(r3, r3seen); + + assertEquals(r3seen, comp.getRevisionSeen(r1)); + assertEquals(r3seen, comp.getRevisionSeen(r2)); + + comp.purge(1); + + assertEquals(null, comp.getRevisionSeen(r1)); + assertEquals(r3seen, comp.getRevisionSeen(r2)); + } + @Test public void uniqueRevision2() throws Exception { List<Thread> threads = new ArrayList<Thread>();