stefan-egli commented on code in PR #1261:
URL: https://github.com/apache/jackrabbit-oak/pull/1261#discussion_r1464610212
##########
oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreSweepTest.java:
##########
@@ -70,12 +81,266 @@ public void before() throws Exception {
ns = createDocumentNodeStore(0);
}
+ @After
+ public void after() throws Exception {
+ if (mf != null) {
+ mf.dispose();
+ mf = null;
+ }
+ }
+
@AfterClass
public static void resetClock() {
Revision.resetClockToDefault();
ClusterNodeInfo.resetClockToDefault();
}
+ interface UpdateCallback {
+ /**
+ * @return true to continue going via this UpdateHandler, false to
stop using
+ * this UpdateHandler
+ */
+ boolean handleUpdate(UpdateOp op);
+ }
+
+ /** limited purpose MongoFixture that allows to pause after a specific
update */
+ MongoFixture pausableMongoDocumentStore(final String targetId,
+ final UpdateCallback updateHandler) {
+ return new MongoFixture() {
+ @Override
+ public DocumentStore createDocumentStore(Builder builder) {
+ try {
+ MongoConnection connection = MongoUtils.getConnection();
+ CountingMongoDatabase db = new CountingMongoDatabase(
+ connection.getDatabase());
+ return new MongoDocumentStore(connection.getMongoClient(),
db,
+ builder) {
+ volatile boolean done;
+
+ @Override
+ public <T extends Document> T findAndUpdate(
+ Collection<T> collection, UpdateOp update) {
+ try {
+ return super.findAndUpdate(collection, update);
+ } finally {
+ updateHandler(targetId, updateHandler, update);
+ }
+ }
+
+ private void updateHandler(final String targetId,
+ final UpdateCallback updateHandler, UpdateOp
update) {
+ if (done) {
+ return;
+ }
+ if (update.getId().equals(targetId)) {
+ if (!updateHandler.handleUpdate(update)) {
+ done = true;
+ return;
+ }
+ }
+ }
+
+ @Override
+ public <T extends Document> T createOrUpdate(
+ Collection<T> collection, UpdateOp update) {
+ try {
+ return super.createOrUpdate(collection,
update);
+ } finally {
+ updateHandler(targetId, updateHandler, update);
+ }
+ }
+
+ @Override
+ public <T extends Document> List<T> createOrUpdate(
+ Collection<T> collection, List<UpdateOp>
updateOps) {
+ try {
+ return super.createOrUpdate(collection,
updateOps);
+ } finally {
+ for (UpdateOp update : updateOps) {
+ updateHandler(targetId, updateHandler,
update);
+ }
+ }
+ }
+ };
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ }
+
+ /**
+ * Test to show-case a race-condition between a collision and
+ * MongoDocumentStore's nodesCache: when a document is read into
+ * the nodesCache shortly before a collision is rolled back,
+ * it runs risk of later making uncommitted changes visible.
+ * <p/>
+ * The test case works as follows:
+ * <ul>
+ * <li>consider clusterId 2 and 4 being active in a cluster</li>
+ * <li>a target path /parent/foo (and sibling /parent/bar) having
+ * formerly been created</li>
+ * <li>clusterId 2: now wants to delete /parent/foo and /parent/bar, and
starts
+ * doing so versus mongo by first setting _deleted:true on those two nodes
+ * (using revision r123456789a-0-2)</li>
+ * <li>before clusterId 2 continues, clusterId 4 comes with a conflicting
update
+ * on /parent/bar (using revision r123456789b-0-4). This update notices
+ * the changes from 2 and leaves a corresponding collision marker (on 0:/
+ * with _collisions.r123456789a-0-2=r123456789b-0-4)</li>
+ * <li>beforre clusterId 4 proceeds, it happens to force a read from
+ * 2:/parent/foo from mongo - this is achieved as a result of
+ * another /parent/foo</li>
+ * <li>the result of the above is clusterId 4 having a state of
2:/parent/foo
+ * in its MongoDocumentStore nodesCache that contains uncommitted
information.
+ * In this test case, that uncommitted information is a deletion. But it
could
+ * be anything else really.</li>
+ * <li>then things continue on both clusterId 2 and 4 (from the previous
+ * test-pause)</li>
+ * <li>then clusterId 2 does another, unrelated change on /parent/bar,
+ * thereby resulting in a newer lastRev (on root and /parent) than the
collision.
+ * Also, it results in a sweepRev that is newer than the collision</li>
+ * <li>when later, clusterId 4 reads /parent/foo, it still finds the
+ * cached 2:/parent/foo document with the uncommitted data - plus it
+ * now has a readRevision/lastRevision that is newer than that - plus
+ * it will resolve that uncommitted data's revision (r123456789a-0-2)
+ * as commitvalue="c", since it is older than the sweepRevision</li>
+ * <li>and thus, clusterId 4 managed to read uncommitted, rolled back data
+ * from an earlier collision</li>
+ * </ul>
+ */
+ @Test
+ @Ignore(value = "OAK-10595")
+ public void cachingUncommittedBeforeCollisionRollback() throws Exception {
+ // two nodes part of the game:
+ // 1 : the main one that starts to do a subtree deletion
+ // 2 : a peer one that gets in between the above and causes a collision
+ // as a result 1 manages to read 2's rolled back (uncommitted) state as
Review Comment:
yes you're right, it's the other way round. (it's also a bit confusing that
this says 1 and 2 while everywhere else 2 and 4 are used.. I'll adjust this as
well)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
