Author: mreutegg
Date: Tue May 7 10:24:15 2019
New Revision: 1858838
URL: http://svn.apache.org/viewvc?rev=1858838&view=rev
Log:
OAK-8300: Revision GC may remove previous document without removing reference
Added:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BackgroundSplitFailureTest.java
(with props)
Modified:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
Modified:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1858838&r1=1858837&r2=1858838&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
Tue May 7 10:24:15 2019
@@ -2367,6 +2367,7 @@ public final class DocumentNodeStore
}
private void backgroundSplit() {
+ Set<Path> invalidatedPaths = new HashSet<>();
RevisionVector head = getHeadRevision();
for (Iterator<String> it = splitCandidates.keySet().iterator();
it.hasNext();) {
String id = it.next();
@@ -2376,6 +2377,30 @@ public final class DocumentNodeStore
}
cleanCollisions(doc, collisionGarbageBatchSize);
for (UpdateOp op : doc.split(this, head, binarySize)) {
+ Path path = doc.getPath();
+ // add an invalidation journal entry, unless the path
+ // already has a pending _lastRev update or an invalidation
+ // entry was already added in this backgroundSplit() call
+ if (unsavedLastRevisions.get(path) == null
+ && invalidatedPaths.add(path)) {
+ // create journal entry for cache invalidation
+ JournalEntry entry =
JOURNAL.newDocument(getDocumentStore());
+ entry.modified(path);
+ Revision r = newRevision().asBranchRevision();
+ UpdateOp journalOp = entry.asUpdateOp(r);
+ if (store.create(JOURNAL, singletonList(journalOp))) {
+ changes.invalidate(singletonList(r));
+ LOG.debug("Journal entry {} created for split of
document {}",
+ journalOp.getId(), path);
+ } else {
+ String msg = "Unable to create journal entry " +
+ journalOp.getId() + " for document
invalidation. " +
+ "Will be retried with next background split " +
+ "operation.";
+ throw new DocumentStoreException(msg);
+ }
+ }
+ // apply the split operations
NodeDocument before = null;
if (!op.isNew() ||
!store.create(Collection.NODES,
Collections.singletonList(op))) {
Modified:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java?rev=1858838&r1=1858837&r2=1858838&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java
Tue May 7 10:24:15 2019
@@ -527,14 +527,32 @@ public final class JournalEntry extends
}
/**
- * @return if this entry has some changes to be pushed
+ * Returns {@code true} if this entry contains any changes to be written
+ * to the journal. The following information is considered a change:
+ * <ul>
+ * <li>Documents that have been recorded as modified with either
+ * {@link #modified(Path)} or {@link #modified(Iterable)}.</li>
+ * <li>Branch commit journal references added with
+ * {@link #branchCommit(Iterable)}.</li>
+ * <li>Documents that must be invalidated and have been recorded
+ * with {@link #invalidate(Iterable)}.</li>
+ * </ul>
+ *
+ * @return if this entry has some changes to be pushed.
*/
boolean hasChanges() {
- return numChangedNodes > 0 || hasBranchCommits;
+ return numChangedNodes > 0
+ || hasBranchCommits
+ || hasInvalidateOnlyReferences();
}
//-----------------------------< internal
>---------------------------------
+ private boolean hasInvalidateOnlyReferences() {
+ String value = (String) get(INVALIDATE_ONLY);
+ return value != null && !value.isEmpty();
+ }
+
private void addInvalidateOnlyTo(final StringSort sort) throws IOException
{
TraversingVisitor v = new TraversingVisitor() {
Added:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BackgroundSplitFailureTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BackgroundSplitFailureTest.java?rev=1858838&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BackgroundSplitFailureTest.java
(added)
+++
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BackgroundSplitFailureTest.java
Tue May 7 10:24:15 2019
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.stats.Clock;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class BackgroundSplitFailureTest {
+
+ @Rule
+ public DocumentMKBuilderProvider builderProvider = new
DocumentMKBuilderProvider();
+
+ private Clock clock = new Clock.Virtual();
+
+ @Before
+ public void before() throws Exception {
+ clock.waitUntil(System.currentTimeMillis());
+ Revision.setClock(clock);
+ ClusterNodeInfo.setClock(clock);
+ }
+
+ @AfterClass
+ public static void after() {
+ Revision.resetClockToDefault();
+ ClusterNodeInfo.resetClockToDefault();
+ }
+
+ @Test
+ public void journalException() throws Exception {
+ DocumentStore store = new MemoryDocumentStore();
+ FailingDocumentStore failingStore = new FailingDocumentStore(store);
+ DocumentNodeStore ns = new DocumentNodeStoreBuilder<>()
+
.setDocumentStore(failingStore).setAsyncDelay(0).clock(clock).build();
+ int clusterId = ns.getClusterId();
+ Path fooPath = new Path(Path.ROOT, "foo");
+ String fooId = Utils.getIdFromPath(fooPath);
+
+ NodeBuilder builder = ns.getRoot().builder();
+ builder.child(fooPath.getName()).setProperty("p", -1);
+ merge(ns, builder);
+ for (int i = 0; i <= NodeDocument.NUM_REVS_THRESHOLD; i++) {
+ builder = ns.getRoot().builder();
+ builder.child(fooPath.getName()).setProperty("p", i);
+ merge(ns, builder);
+ }
+
+ // shut down without running background ops
+ failingStore.fail().after(0).eternally();
+ try {
+ ns.dispose();
+ fail("dispose is expected to fail");
+ } catch (Exception e) {
+ // expected
+ }
+
+ // must not have previous documents yet
+ NodeDocument foo = store.find(Collection.NODES, fooId);
+ assertNotNull(foo);
+ assertEquals(0, foo.getPreviousRanges().size());
+
+ // start again with test store
+ final AtomicBoolean falseOnJournalEntryCreate = new AtomicBoolean();
+ DocumentStore testStore = new DocumentStoreWrapper(store) {
+ @Override
+ public <T extends Document> boolean create(Collection<T>
collection,
+ List<UpdateOp>
updateOps) {
+ if (collection == Collection.JOURNAL
+ && falseOnJournalEntryCreate.get()) {
+ return false;
+ }
+ return super.create(collection, updateOps);
+ }
+ };
+ ns = builderProvider.newBuilder().setClusterId(clusterId)
+
.setDocumentStore(testStore).setAsyncDelay(0).clock(clock).build();
+ ns.addSplitCandidate(Utils.getIdFromPath(new Path(Path.ROOT, "foo")));
+ falseOnJournalEntryCreate.set(true);
+ try {
+ ns.runBackgroundOperations();
+ fail("background operations are expected to fail");
+ } catch (DocumentStoreException e) {
+ assertThat(e.getMessage(), containsString("Unable to create
journal entry"));
+ }
+
+ // must still not have previous documents
+ foo = store.find(Collection.NODES, fooId);
+ assertNotNull(foo);
+ assertEquals(0, foo.getPreviousRanges().size());
+ assertTrue(ns.getSplitCandidates().contains(fooId));
+
+ falseOnJournalEntryCreate.set(false);
+ ns.runBackgroundOperations();
+
+ // now there must be a split document
+ foo = store.find(Collection.NODES, fooId);
+ assertNotNull(foo);
+ assertEquals(1, foo.getPreviousRanges().size());
+ assertFalse(ns.getSplitCandidates().contains(fooId));
+ }
+}
Propchange:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/BackgroundSplitFailureTest.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java?rev=1858838&r1=1858837&r2=1858838&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java
Tue May 7 10:24:15 2019
@@ -398,6 +398,20 @@ public class JournalEntryTest {
}
@Test
+ public void invalidationPathsMarksChanges() {
+ DocumentStore store = new MemoryDocumentStore();
+ JournalEntry entry = JOURNAL.newDocument(store);
+
+ assertFalse("Incorrect hasChanges", entry.hasChanges());
+
+ entry.invalidate(Collections.emptyList());
+ assertFalse("Incorrect hasChanges", entry.hasChanges());
+
+
entry.invalidate(Collections.singleton(Revision.fromString("r123-0-1")));
+ assertTrue("Incorrect hasChanges", entry.hasChanges());
+ }
+
+ @Test
public void emptyBranchCommit() {
DocumentStore store = new MemoryDocumentStore();
JournalEntry entry = JOURNAL.newDocument(store);
Modified:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java?rev=1858838&r1=1858837&r2=1858838&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
Tue May 7 10:24:15 2019
@@ -83,7 +83,6 @@ import org.apache.jackrabbit.oak.stats.C
import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -880,7 +879,6 @@ public class VersionGarbageCollectorIT {
assertEquals(value,
store.getRoot().getChildNode("foo").getString("prop"));
}
- @Ignore("OAK-8300")
@Test
public void gcOnStaleDocument() throws Exception {
assumeTrue(fixture.hasSinglePersistence());
