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 re...@apache.org on 2017/02/23 13:29:28 UTC
svn commit: r1784128 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
Author: reschke
Date: Thu Feb 23 13:29:28 2017
New Revision: 1784128
URL: http://svn.apache.org/viewvc?rev=1784128&view=rev
Log:
OAK-5704: VersionGC: reset _deletedOnce for documents that have been resurrected
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1784128&r1=1784127&r2=1784128&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Thu Feb 23 13:29:28 2017
@@ -172,12 +172,13 @@ public final class NodeDocument extends
private static final String DELETED = "_deleted";
/**
- * Flag indicating that whether this node was ever deleted.
- * Its just used as a hint. If set to true then it indicates that
- * node was once deleted.
- *
- * <p>Note that a true value does not mean that node should be considered
- * deleted as it might have been resurrected in later revision</p>
+ * Flag indicating that whether this node was ever deleted. Its just used as
+ * a hint. If set to true then it indicates that node was once deleted.
+ * <p>
+ * Note that a true value does not mean that node should be considered
+ * deleted as it might have been resurrected in later revision. Further note
+ * that it might get reset by maintenance tasks once they discover that it
+ * indeed was resurrected.
*/
public static final String DELETED_ONCE = "_deletedOnce";
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java?rev=1784128&r1=1784127&r2=1784128&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java Thu Feb 23 13:29:28 2017
@@ -64,6 +64,7 @@ import static org.apache.jackrabbit.oak.
public class VersionGarbageCollector {
//Kept less than MongoDocumentStore.IN_CLAUSE_BATCH_SIZE to avoid re-partitioning
private static final int DELETE_BATCH_SIZE = 450;
+ private static final int UPDATE_BATCH_SIZE = 450;
private static final int PROGRESS_BATCH_SIZE = 10000;
private static final Key KEY_MODIFIED = new Key(MODIFIED_IN_SECS, null);
private final DocumentNodeStore nodeStore;
@@ -119,10 +120,12 @@ public class VersionGarbageCollector {
int deletedLeafDocGCCount;
int splitDocGCCount;
int intermediateSplitDocGCCount;
+ int updateResurrectedGCCount;
final Stopwatch collectDeletedDocs = Stopwatch.createUnstarted();
final Stopwatch deleteDeletedDocs = Stopwatch.createUnstarted();
final Stopwatch collectAndDeleteSplitDocs = Stopwatch.createUnstarted();
final Stopwatch sortDocIds = Stopwatch.createUnstarted();
+ final Stopwatch updateResurrectedDocuments = Stopwatch.createUnstarted();
@Override
public String toString() {
@@ -130,6 +133,7 @@ public class VersionGarbageCollector {
"ignoredGCDueToCheckPoint=" + ignoredGCDueToCheckPoint +
", canceled=" + canceled +
", deletedDocGCCount=" + deletedDocGCCount + " (of which leaf: " + deletedLeafDocGCCount + ")" +
+ ", updateResurrectedGCCount=" + updateResurrectedGCCount +
", splitDocGCCount=" + splitDocGCCount +
", intermediateSplitDocGCCount=" + intermediateSplitDocGCCount +
", timeToCollectDeletedDocs=" + collectDeletedDocs +
@@ -145,7 +149,8 @@ public class VersionGarbageCollector {
COLLECTING,
DELETING,
SORTING,
- SPLITS_CLEANUP
+ SPLITS_CLEANUP,
+ UPDATING
}
/**
@@ -169,6 +174,7 @@ public class VersionGarbageCollector {
this.watches.put(GCPhase.DELETING, stats.deleteDeletedDocs);
this.watches.put(GCPhase.SORTING, stats.sortDocIds);
this.watches.put(GCPhase.SPLITS_CLEANUP, stats.collectAndDeleteSplitDocs);
+ this.watches.put(GCPhase.UPDATING, stats.updateResurrectedDocuments);
this.canceled = canceled;
}
@@ -311,6 +317,12 @@ public class VersionGarbageCollector {
phases.stop(GCPhase.DELETING);
}
}
+ if (gc.hasRescurrectUpdateBatch()) {
+ if (phases.start(GCPhase.UPDATING)) {
+ gc.updateResurrectedDocuments(phases.stats);
+ phases.stop(GCPhase.UPDATING);
+ }
+ }
}
} finally {
Utils.closeIfCloseable(itr);
@@ -318,23 +330,26 @@ public class VersionGarbageCollector {
phases.stop(GCPhase.COLLECTING);
}
- if (gc.getNumDocuments() == 0){
- return;
- }
+ if (gc.getNumDocuments() != 0) {
+ if (phases.start(GCPhase.DELETING)) {
+ gc.removeLeafDocuments(phases.stats);
+ phases.stop(GCPhase.DELETING);
+ }
- if (phases.start(GCPhase.DELETING)) {
- gc.removeLeafDocuments(phases.stats);
- phases.stop(GCPhase.DELETING);
- }
+ if (phases.start(GCPhase.SORTING)) {
+ gc.ensureSorted();
+ phases.stop(GCPhase.SORTING);
+ }
- if (phases.start(GCPhase.SORTING)) {
- gc.ensureSorted();
- phases.stop(GCPhase.SORTING);
+ if (phases.start(GCPhase.DELETING)) {
+ gc.removeDocuments(phases.stats);
+ phases.stop(GCPhase.DELETING);
+ }
}
- if (phases.start(GCPhase.DELETING)) {
- gc.removeDocuments(phases.stats);
- phases.stop(GCPhase.DELETING);
+ if (phases.start(GCPhase.UPDATING)) {
+ gc.updateResurrectedDocuments(phases.stats);
+ phases.stop(GCPhase.UPDATING);
}
} finally {
gc.close();
@@ -350,6 +365,7 @@ public class VersionGarbageCollector {
private final RevisionVector headRevision;
private final AtomicBoolean cancel;
private final List<String> leafDocIdsToDelete = Lists.newArrayList();
+ private final List<String> resurrectedIds = Lists.newArrayList();
private final StringSort docIdsToDelete = newStringSort();
private final StringSort prevDocIdsToDelete = newStringSort();
private final Set<String> exclude = Sets.newHashSet();
@@ -400,6 +416,8 @@ public class VersionGarbageCollector {
addDocument(id);
addPreviousDocuments(previousDocs);
}
+ } else {
+ addNonDeletedDocument(id);
}
}
@@ -422,6 +440,10 @@ public class VersionGarbageCollector {
return leafDocIdsToDelete.size() >= DELETE_BATCH_SIZE;
}
+ boolean hasRescurrectUpdateBatch() {
+ return resurrectedIds.size() >= UPDATE_BATCH_SIZE;
+ }
+
void removeLeafDocuments(VersionGCStats stats) throws IOException {
int removeCount = removeDeletedDocuments(getLeafDocIdsToDelete(), "(leaf)");
leafDocIdsToDelete.clear();
@@ -429,6 +451,12 @@ public class VersionGarbageCollector {
stats.deletedDocGCCount += removeCount;
}
+ void updateResurrectedDocuments(VersionGCStats stats) throws IOException {
+ int updateCount = resetDeletedOnce(resurrectedIds);
+ resurrectedIds.clear();
+ stats.updateResurrectedGCCount += updateCount;
+ }
+
public void close() {
try {
docIdsToDelete.close();
@@ -481,6 +509,10 @@ public class VersionGarbageCollector {
leafDocIdsToDelete.add(id);
}
+ private void addNonDeletedDocument(String id) throws IOException {
+ resurrectedIds.add(id);
+ }
+
private long getNumPreviousDocuments() {
return prevDocIdsToDelete.getSize() - exclude.size();
}
@@ -528,15 +560,8 @@ public class VersionGarbageCollector {
while (idListItr.hasNext() && !cancel.get()) {
Map<String, Map<Key, Condition>> deletionBatch = Maps.newLinkedHashMap();
for (String s : idListItr.next()) {
- int idx = s.lastIndexOf('/');
- String id = s.substring(0, idx);
- long modified = -1;
- try {
- modified = Long.parseLong(s.substring(idx + 1));
- } catch (NumberFormatException e) {
- log.warn("Invalid _modified {} for {}", s.substring(idx + 1), id);
- }
- deletionBatch.put(id, singletonMap(KEY_MODIFIED, newEqualsCondition(modified)));
+ Map.Entry<String, Long> parsed = parseEntry(s);
+ deletionBatch.put(parsed.getKey(), singletonMap(KEY_MODIFIED, newEqualsCondition(parsed.getValue())));
}
if (log.isDebugEnabled()) {
@@ -572,6 +597,30 @@ public class VersionGarbageCollector {
return deletedCount;
}
+ private int resetDeletedOnce(List<String> resurrectedDocuments) throws IOException {
+ log.info("Proceeding to reset [{}] _deletedOnce flags", resurrectedDocuments.size());
+
+ int updateCount = 0;
+ for (String s : resurrectedDocuments) {
+ if (!cancel.get()) {
+ Map.Entry<String, Long> parsed = parseEntry(s);
+ try {
+ UpdateOp up = new UpdateOp(parsed.getKey(), false);
+ up.equals(MODIFIED_IN_SECS, parsed.getValue());
+ up.remove(NodeDocument.DELETED_ONCE);
+ NodeDocument r = ds.findAndUpdate(Collection.NODES, up);
+ if (r != null) {
+ updateCount += 1;
+ }
+ }
+ catch (DocumentStoreException ex) {
+ log.warn("updating {}: {}", parsed.getKey(), ex.getMessage());
+ }
+ }
+ }
+ return updateCount;
+ }
+
private int removeDeletedPreviousDocuments() throws IOException {
log.info("Proceeding to delete [{}] previous documents", getNumPreviousDocuments());
@@ -610,6 +659,29 @@ public class VersionGarbageCollector {
sorted = true;
}
}
+
+ /**
+ * Parses an id/modified entry and returns the two components as a
+ * Map.Entry.
+ *
+ * @param entry the id/modified String.
+ * @return the parsed components.
+ * @throws IllegalArgumentException if the entry is malformed.
+ */
+ private Map.Entry<String, Long> parseEntry(String entry) throws IllegalArgumentException {
+ int idx = entry.lastIndexOf('/');
+ if (idx == -1) {
+ throw new IllegalArgumentException(entry);
+ }
+ String id = entry.substring(0, idx);
+ long modified;
+ try {
+ modified = Long.parseLong(entry.substring(idx + 1));
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(entry);
+ }
+ return Maps.immutableEntry(id, modified);
+ }
}
@Nonnull
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java?rev=1784128&r1=1784127&r2=1784128&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGCDeletionTest.java Thu Feb 23 13:29:28 2017
@@ -19,6 +19,16 @@
package org.apache.jackrabbit.oak.plugins.document;
+import static java.util.concurrent.Executors.newSingleThreadExecutor;
+import static java.util.concurrent.TimeUnit.HOURS;
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
@@ -36,6 +46,7 @@ import com.google.common.collect.Lists;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.plugins.document.VersionGarbageCollector.VersionGCStats;
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
import org.apache.jackrabbit.oak.spi.commit.EmptyHook;
import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
@@ -44,16 +55,8 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.stats.Clock;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
-import static java.util.concurrent.Executors.newSingleThreadExecutor;
-import static java.util.concurrent.TimeUnit.HOURS;
-import static java.util.concurrent.TimeUnit.MINUTES;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.fail;
-
public class VersionGCDeletionTest {
private Clock clock;
@@ -120,6 +123,56 @@ public class VersionGCDeletionTest {
}
@Test
+ public void leaveResurrectedNodesAlone() throws Exception{
+ TestDocumentStore ts = new TestDocumentStore();
+ store = new DocumentMK.Builder()
+ .clock(clock)
+ .setDocumentStore(ts)
+ .setAsyncDelay(0)
+ .getNodeStore();
+
+ //Baseline the clock
+ clock.waitUntil(Revision.getCurrentTimestamp());
+
+ String id = Utils.getIdFromPath("/x");
+
+ NodeBuilder b1 = store.getRoot().builder();
+ b1.child("x");
+ store.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+
+ // Remove x
+ NodeBuilder b2 = store.getRoot().builder();
+ b2.child("x").remove();
+ store.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ store.runBackgroundOperations();
+
+ NodeDocument d2 = ts.find(Collection.NODES, id, 0);
+ assertTrue(d2.wasDeletedOnce());
+
+ // Re-add x
+ NodeBuilder b3 = store.getRoot().builder();
+ b3.child("x");
+ store.merge(b3, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ store.runBackgroundOperations();
+
+ NodeDocument d3 = ts.find(Collection.NODES, id, 0);
+ assertTrue(d3.wasDeletedOnce());
+
+ long maxAge = 1; //hours
+ long delta = TimeUnit.MINUTES.toMillis(10);
+
+ // 3. Check that resurrected doc does not get collected post maxAge
+ clock.waitUntil(clock.getTime() + HOURS.toMillis(maxAge * 2) + delta);
+ VersionGarbageCollector gc = store.getVersionGarbageCollector();
+
+ VersionGCStats stats = gc.gc(maxAge * 2, HOURS);
+ assertEquals(1, stats.updateResurrectedGCCount);
+ NodeDocument d4 = ts.find(Collection.NODES, id, 0);
+ assertNotNull(d4);
+ assertFalse(d4.wasDeletedOnce());
+ }
+
+ @Test
public void deleteLargeNumber() throws Exception{
int noOfDocsToDelete = 10000;
DocumentStore ts = new MemoryDocumentStore();
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java?rev=1784128&r1=1784127&r2=1784128&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollectorIT.java Thu Feb 23 13:29:28 2017
@@ -206,7 +206,7 @@ public class VersionGarbageCollectorIT {
stats = gc.gc(maxAge*2, HOURS);
assertEquals(0, stats.deletedDocGCCount);
assertEquals(0, stats.deletedLeafDocGCCount);
-
+ assertEquals(1, stats.updateResurrectedGCCount);
}
@Test