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 md...@apache.org on 2016/04/20 12:05:35 UTC
svn commit: r1740101 - in /jackrabbit/oak/trunk/oak-segment-next/src:
main/java/org/apache/jackrabbit/oak/plugins/backup/
main/java/org/apache/jackrabbit/oak/plugins/segment/
main/java/org/apache/jackrabbit/oak/plugins/segment/file/
test/java/org/apach...
Author: mduerig
Date: Wed Apr 20 10:05:34 2016
New Revision: 1740101
URL: http://svn.apache.org/viewvc?rev=1740101&view=rev
Log:
OAK-3348: Cross gc sessions might introduce references to pre-compacted segments
RIP Compactor: replace compactor with write node and deduplication cache
Removed:
jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Compactor.java
Modified:
jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreBackup.java
jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreRestore.java
jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentTracker.java
jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactorTest.java
jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStoreIT.java
Modified: jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreBackup.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreBackup.java?rev=1740101&r1=1740100&r2=1740101&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreBackup.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreBackup.java Wed Apr 20 10:05:34 2016
@@ -23,7 +23,7 @@ import static com.google.common.base.Pre
import java.io.File;
import java.io.IOException;
-import org.apache.jackrabbit.oak.plugins.segment.Compactor;
+import com.google.common.base.Stopwatch;
import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeState;
import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeStore;
import org.apache.jackrabbit.oak.plugins.segment.file.FileStore;
@@ -33,8 +33,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Stopwatch;
-
public class FileStoreBackup {
private static final Logger log = LoggerFactory
@@ -55,11 +53,13 @@ public class FileStoreBackup {
FileStore backup = builder.build();
try {
SegmentNodeState state = backup.getHead();
- Compactor compactor = new Compactor(backup.getTracker());
+ // This is allows us to decouple and fix problems for online compaction independent
+ // of backup / restore.
+// Compactor compactor = new Compactor(backup.getTracker());
// compactor.setDeepCheckLargeBinaries(true);
// compactor.setContentEqualityCheck(true);
- SegmentNodeState after = compactor.compact(state, current, state);
- backup.setHead(state, after);
+// SegmentNodeState after = compactor.compact(state, current, state);
+// backup.setHead(state, after);
} finally {
backup.close();
}
Modified: jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreRestore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreRestore.java?rev=1740101&r1=1740100&r2=1740101&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreRestore.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/backup/FileStoreRestore.java Wed Apr 20 10:05:34 2016
@@ -22,7 +22,6 @@ import java.io.File;
import java.io.IOException;
import com.google.common.base.Stopwatch;
-import org.apache.jackrabbit.oak.plugins.segment.Compactor;
import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeState;
import org.apache.jackrabbit.oak.plugins.segment.file.FileStore;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
@@ -51,10 +50,13 @@ public class FileStoreRestore {
FileStore store = FileStore.builder(destination).build();
SegmentNodeState current = store.getHead();
try {
- Compactor compactor = new Compactor(store.getTracker());
- SegmentNodeState after = compactor.compact(current,
- restore.getHead(), current);
- store.setHead(current, after);
+ // This is allows us to decouple and fix problems for online compaction independent
+ // of backup / restore.
+ // compactor.setDeepCheckLargeBinaries(true);
+// Compactor compactor = new Compactor(store.getTracker());
+// SegmentNodeState after = compactor.compact(current,
+// restore.getHead(), current);
+// store.setHead(current, after);
} finally {
restore.close();
store.close();
Modified: jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentTracker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentTracker.java?rev=1740101&r1=1740100&r2=1740101&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentTracker.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentTracker.java Wed Apr 20 10:05:34 2016
@@ -246,7 +246,9 @@ public class SegmentTracker {
segmentCache.put(id, segment, segment.size());
}
- int getGcGen() {
+ // See also the comments in FileStore regarding initialisation and
+ // cyclic dependencies.
+ public int getGcGen() {
if (store instanceof FileStore) {
return ((FileStore) store).getGcGen();
} else {
Modified: jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java?rev=1740101&r1=1740100&r2=1740101&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java Wed Apr 20 10:05:34 2016
@@ -64,10 +64,11 @@ import com.google.common.base.Stopwatch;
import com.google.common.base.Supplier;
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.plugins.blob.BlobStoreBlob;
-import org.apache.jackrabbit.oak.plugins.segment.Compactor;
+import org.apache.jackrabbit.oak.plugins.segment.RecordCache;
import org.apache.jackrabbit.oak.plugins.segment.RecordCache.DeduplicationCache;
import org.apache.jackrabbit.oak.plugins.segment.RecordId;
import org.apache.jackrabbit.oak.plugins.segment.Segment;
+import org.apache.jackrabbit.oak.plugins.segment.SegmentBufferWriter;
import org.apache.jackrabbit.oak.plugins.segment.SegmentGraph.SegmentGraphVisitor;
import org.apache.jackrabbit.oak.plugins.segment.SegmentId;
import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeState;
@@ -76,6 +77,7 @@ import org.apache.jackrabbit.oak.plugins
import org.apache.jackrabbit.oak.plugins.segment.SegmentStore;
import org.apache.jackrabbit.oak.plugins.segment.SegmentTracker;
import org.apache.jackrabbit.oak.plugins.segment.SegmentVersion;
+import org.apache.jackrabbit.oak.plugins.segment.SegmentWriter;
import org.apache.jackrabbit.oak.plugins.segment.compaction.CompactionStrategy;
import org.apache.jackrabbit.oak.spi.blob.BlobStore;
import org.apache.jackrabbit.oak.spi.gc.GCMonitor;
@@ -998,9 +1000,19 @@ public class FileStore implements Segmen
"You must set a compactionStrategy before calling compact");
gcMonitor.info("TarMK GC #{}: compaction started, strategy={}", gcCount, compactionStrategy);
Stopwatch watch = Stopwatch.createStarted();
- Supplier<Boolean> compactionCanceled = newCancelCompactionCondition();
- DeduplicationCache<String> nodeCache = new DeduplicationCache<String>(1000000, 20);
- Compactor compactor = new Compactor(tracker, nodeCache, compactionCanceled);
+
+ final DeduplicationCache<String> nodeCache = new DeduplicationCache<String>(1000000, 20);
+
+ int gcGeneration = tracker.getGcGen() + 1;
+ SegmentWriter writer = new SegmentWriter(this, tracker.getSegmentVersion(),
+ new SegmentBufferWriter(this, tracker.getSegmentVersion(), "c", gcGeneration),
+ new RecordCache<String>() {
+ @Override
+ protected Cache<String> getCache(int generation) {
+ return nodeCache;
+ }
+ });
+
SegmentNodeState before = getHead();
long existing = before.getChildNode(SegmentNodeStore.CHECKPOINTS)
.getChildNodeCount(Long.MAX_VALUE);
@@ -1010,15 +1022,10 @@ public class FileStore implements Segmen
gcCount, existing);
}
- SegmentNodeState after = compactor.compact(EMPTY_NODE, before, EMPTY_NODE);
+ SegmentNodeState after = compact(writer, before);
gcMonitor.info("TarMK GC #{}: compacted {} to {}",
gcCount, before.getRecordId(), after.getRecordId());
- if (compactionCanceled.get()) {
- gcMonitor.warn("TarMK GC #{}: compaction canceled: {}", gcCount, compactionCanceled);
- return;
- }
-
try {
int cycles = 0;
boolean success = false;
@@ -1030,18 +1037,13 @@ public class FileStore implements Segmen
gcMonitor.info("TarMK GC #{}: compaction detected concurrent commits while compacting. " +
"Compacting these commits. Cycle {}", gcCount, cycles);
SegmentNodeState head = getHead();
- after = compactor.compact(before, head, after);
+ after = compact(writer, head);
gcMonitor.info("TarMK GC #{}: compacted {} against {} to {}",
gcCount, head.getRecordId(), before.getRecordId(), after.getRecordId());
before = head;
-
- if (compactionCanceled.get()) {
- gcMonitor.warn("TarMK GC #{}: compaction canceled: {}", gcCount, compactionCanceled);
- return;
- }
}
if (success) {
- tracker.getWriter().addCachedNodes(compactor.getGCGeneration(), nodeCache);
+ tracker.getWriter().addCachedNodes(gcGeneration, nodeCache);
tracker.clearSegmentIdTables(compactionStrategy);
gcMonitor.compacted(new long[0], new long[0], new long[0]);
} else {
@@ -1049,7 +1051,7 @@ public class FileStore implements Segmen
gcCount, cycles - 1);
if (compactionStrategy.getForceAfterFail()) {
gcMonitor.info("TarMK GC #{}: compaction force compacting remaining commits", gcCount);
- if (!forceCompact(before, after, compactor)) {
+ if (!forceCompact(writer)) {
gcMonitor.warn("TarMK GC #{}: compaction failed to force compact remaining commits. " +
"Most likely compaction didn't get exclusive access to the store.", gcCount);
}
@@ -1066,12 +1068,17 @@ public class FileStore implements Segmen
}
}
- private boolean forceCompact(NodeState before, SegmentNodeState onto, Compactor compactor)
- throws InterruptedException, IOException {
+ private static SegmentNodeState compact(SegmentWriter writer, NodeState node) throws IOException {
+ SegmentNodeState compacted = writer.writeNode(node);
+ writer.flush();
+ return compacted;
+ }
+
+ private boolean forceCompact(SegmentWriter writer) throws InterruptedException, IOException {
if (rwLock.writeLock().tryLock(compactionStrategy.getLockWaitTime(), TimeUnit.SECONDS)) {
try {
SegmentNodeState head = getHead();
- return setHead(head, compactor.compact(before, head, onto));
+ return setHead(head, compact(writer, head));
} finally {
rwLock.writeLock().unlock();
}
Modified: jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactorTest.java?rev=1740101&r1=1740100&r2=1740101&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactorTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactorTest.java Wed Apr 20 10:05:34 2016
@@ -16,12 +16,8 @@
*/
package org.apache.jackrabbit.oak.plugins.segment;
-import static org.junit.Assert.assertFalse;
-
import java.io.IOException;
-import com.google.common.base.Suppliers;
-import junit.framework.Assert;
import org.apache.jackrabbit.oak.Oak;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.plugins.segment.memory.MemoryStore;
@@ -33,7 +29,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStore;
import org.junit.After;
import org.junit.Before;
-import org.junit.Test;
public class CompactorTest {
@@ -49,37 +44,6 @@ public class CompactorTest {
segmentStore.close();
}
- @Test
- public void testCompactor() throws Exception {
- NodeStore store = SegmentNodeStore.builder(segmentStore).build();
- init(store);
-
- Compactor compactor = new Compactor(segmentStore.getTracker());
- addTestContent(store, 0);
-
- NodeState initial = store.getRoot();
- SegmentNodeState after = compactor
- .compact(initial, store.getRoot(), initial);
- Assert.assertEquals(store.getRoot(), after);
-
- addTestContent(store, 1);
- after = compactor.compact(initial, store.getRoot(), initial);
- Assert.assertEquals(store.getRoot(), after);
- }
-
- @Test
- public void testCancel() throws Throwable {
-
- // Create a Compactor that will cancel itself as soon as possible. The
- // early cancellation is the reason why the returned SegmentNodeState
- // doesn't have the child named "b".
-
- NodeStore store = SegmentNodeStore.builder(segmentStore).build();
- Compactor compactor = new Compactor(segmentStore.getTracker(), Suppliers.ofInstance(true));
- SegmentNodeState sns = compactor.compact(store.getRoot(), addChild(store.getRoot(), "b"), store.getRoot());
- assertFalse(sns.hasChildNode("b"));
- }
-
private NodeState addChild(NodeState current, String name) {
NodeBuilder builder = current.builder();
builder.child(name);
Modified: jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStoreIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStoreIT.java?rev=1740101&r1=1740100&r2=1740101&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStoreIT.java (original)
+++ jackrabbit/oak/trunk/oak-segment-next/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStoreIT.java Wed Apr 20 10:05:34 2016
@@ -20,7 +20,6 @@ import static com.google.common.collect.
import static com.google.common.collect.Sets.newTreeSet;
import static org.apache.jackrabbit.oak.commons.FixturesHelper.Fixture.SEGMENT_MK;
import static org.apache.jackrabbit.oak.commons.FixturesHelper.getFixtures;
-import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -38,11 +37,8 @@ import java.util.Random;
import com.google.common.base.Strings;
import org.apache.jackrabbit.oak.api.Blob;
-import org.apache.jackrabbit.oak.plugins.segment.Compactor;
import org.apache.jackrabbit.oak.plugins.segment.RecordId;
import org.apache.jackrabbit.oak.plugins.segment.Segment;
-import org.apache.jackrabbit.oak.plugins.segment.SegmentBlob;
-import org.apache.jackrabbit.oak.plugins.segment.SegmentBufferWriter;
import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeBuilder;
import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeState;
import org.apache.jackrabbit.oak.plugins.segment.SegmentWriter;
@@ -102,73 +98,6 @@ public class FileStoreIT {
store.close();
}
- @Test
- @Ignore
- public void testCompaction() throws IOException {
- int largeBinarySize = 10 * 1024 * 1024;
-
- FileStore store = FileStore.builder(getFileStoreFolder()).withMaxFileSize(1).withMemoryMapping(false).build();
- SegmentWriter writer = store.getTracker().getWriter();
-
- SegmentNodeState base = store.getHead();
- SegmentNodeBuilder builder = base.builder();
- byte[] data = new byte[largeBinarySize];
- new Random().nextBytes(data);
- SegmentBlob blob = writer.writeStream(new ByteArrayInputStream(data));
- builder.setProperty("foo", blob);
- builder.getNodeState(); // write the blob reference to the segment
- builder.setProperty("foo", "bar");
- SegmentNodeState head = builder.getNodeState();
- assertTrue(store.setHead(base, head));
- assertEquals("bar", store.getHead().getString("foo"));
-
- Compactor compactor = new Compactor(store.getTracker());
- SegmentNodeState compacted =
- compactor.compact(EMPTY_NODE, head, EMPTY_NODE);
- store.close();
-
- // First simulate the case where during compaction a reference to the
- // older segments is added to a segment that the compactor is writing
- store = FileStore.builder(getFileStoreFolder()).withMaxFileSize(1).withMemoryMapping(false).build();
- head = store.getHead();
- assertTrue(store.size() > largeBinarySize);
- builder = head.builder();
- builder.setChildNode("old", head); // reference to pre-compacted state
- builder.getNodeState();
- assertTrue(store.setHead(head, compacted));
- store.close();
-
- // In this case the revision cleanup is unable to reclaim the old data
- store = FileStore.builder(getFileStoreFolder()).withMaxFileSize(1).withMemoryMapping(false).build();
- assertTrue(store.size() > largeBinarySize);
- store.cleanup();
- assertTrue(store.size() > largeBinarySize);
- store.close();
-
- // Now we do the same thing, but let the compactor use a different
- // SegmentWriter
- store = FileStore.builder(getFileStoreFolder()).withMaxFileSize(1).withMemoryMapping(false).build();
- head = store.getHead();
- assertTrue(store.size() > largeBinarySize);
- writer = new SegmentWriter(store, store.getTracker().getSegmentVersion(),
- new SegmentBufferWriter(store, store.getTracker().getSegmentVersion(), ""));
- compactor = new Compactor(store.getTracker());
- compacted = compactor.compact(EMPTY_NODE, head, EMPTY_NODE);
- builder = head.builder();
- builder.setChildNode("old", head); // reference to pre-compacted state
- builder.getNodeState();
- writer.flush();
- assertTrue(store.setHead(head, compacted));
- store.close();
-
- // Revision cleanup is now able to reclaim the extra space (OAK-1932)
- store = FileStore.builder(getFileStoreFolder()).withMaxFileSize(1).withMemoryMapping(false).build();
- assertTrue(store.size() > largeBinarySize);
- store.cleanup();
- assertTrue(store.size() < largeBinarySize);
- store.close();
- }
-
@Test
public void testRecovery() throws IOException {
FileStore store = FileStore.builder(getFileStoreFolder()).withMaxFileSize(1).withMemoryMapping(false).build();