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 2015/07/29 09:33:47 UTC

svn commit: r1693195 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/segment/ main/java/org/apache/jackrabbit/oak/plugins/segment/file/ test/java/org/apache/jackrabbit/oak/plugins/segment/

Author: mduerig
Date: Wed Jul 29 07:33:47 2015
New Revision: 1693195

URL: http://svn.apache.org/r1693195
Log:
OAK-3139: SNFE in persisted comapation map when using CLEAN_OLD
Create segment writer on the fly to avoid the underlying getting gc-ed

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Compactor.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/PersistedCompactionMap.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/package-info.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/package-info.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionAndCleanupTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionMapTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/PartialCompactionMapTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Compactor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Compactor.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Compactor.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Compactor.java Wed Jul 29 07:33:47 2015
@@ -16,11 +16,11 @@
  */
 package org.apache.jackrabbit.oak.plugins.segment;
 
-import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
 import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 import static org.apache.jackrabbit.oak.api.Type.BINARIES;
 import static org.apache.jackrabbit.oak.api.Type.BINARY;
+import static org.apache.jackrabbit.oak.commons.PathUtils.concat;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -28,6 +28,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
+import com.google.common.hash.Hashing;
 import org.apache.jackrabbit.oak.api.Blob;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
@@ -36,14 +37,14 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState;
 import org.apache.jackrabbit.oak.plugins.memory.MultiBinaryPropertyState;
 import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.plugins.segment.compaction.CompactionStrategy;
+import org.apache.jackrabbit.oak.plugins.segment.file.FileStore;
 import org.apache.jackrabbit.oak.spi.state.ApplyDiff;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.hash.Hashing;
-
 /**
  * Tool for compacting segments.
  */
@@ -78,17 +79,19 @@ public class Compactor {
     private final boolean cloneBinaries;
 
     public Compactor(SegmentWriter writer) {
-        this(writer, null, false);
+        this.writer = writer;
+        this.map = new InMemoryCompactionMap(writer.getTracker());
+        this.cloneBinaries = false;
     }
 
-    public Compactor(SegmentWriter writer, SegmentWriter mapWriter, boolean cloneBinaries) {
-        this.writer = writer;
-        if (mapWriter != null) {
-            this.map = new PersistedCompactionMap(mapWriter);
+    public Compactor(FileStore store, CompactionStrategy compactionStrategy) {
+        this.writer = store.createSegmentWriter();
+        if (compactionStrategy.getPersistCompactionMap()) {
+            this.map = new PersistedCompactionMap(store);
         } else {
             this.map = new InMemoryCompactionMap(writer.getTracker());
         }
-        this.cloneBinaries = cloneBinaries;
+        this.cloneBinaries = compactionStrategy.cloneBinaries();
     }
 
     protected SegmentNodeBuilder process(NodeState before, NodeState after) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/PersistedCompactionMap.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/PersistedCompactionMap.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/PersistedCompactionMap.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/PersistedCompactionMap.java Wed Jul 29 07:33:47 2015
@@ -34,6 +34,8 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.jackrabbit.oak.plugins.segment.file.FileStore;
+
 /**
  * A {@code PartialCompactionMap} implementation persisting its entries
  * to segments.
@@ -61,14 +63,15 @@ public class PersistedCompactionMap impl
     public static final String PERSISTED_COMPACTION_MAP = "PersistedCompactionMap";
 
     private final TreeMap<UUID, RecordIdMap> recent = newTreeMap();
-    private final SegmentWriter writer;
+
+    private final FileStore store;
 
     private int recentCount;
     private long recordCount;
     private MapRecord entries;
 
-    PersistedCompactionMap(@Nonnull SegmentWriter writer) {
-        this.writer = writer;
+    PersistedCompactionMap(@Nonnull FileStore store) {
+        this.store = store;
     }
 
     @Override
@@ -97,7 +100,7 @@ public class PersistedCompactionMap impl
             return recordId;
         }
 
-        return get(writer.getTracker(), entries, uuid, offset);
+        return get(store.getTracker(), entries, uuid, offset);
     }
 
     @CheckForNull
@@ -172,6 +175,7 @@ public class PersistedCompactionMap impl
             return;
         }
 
+        SegmentWriter writer = store.createSegmentWriter();
         Map<String, RecordId> segmentIdMap = newHashMap();
         for (Entry<UUID, RecordIdMap> recentEntry : recent.entrySet()) {
             UUID uuid = recentEntry.getKey();

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java Wed Jul 29 07:33:47 2015
@@ -703,6 +703,13 @@ public class FileStore implements Segmen
     }
 
     /**
+     * @return  a new {@link SegmentWriter} instance for writing to this store.
+     */
+    public SegmentWriter createSegmentWriter() {
+        return new SegmentWriter(this, tracker, getVersion());
+    }
+
+    /**
      * Copy every referenced record in data (non-bulk) segments. Bulk segments
      * are fully kept (they are only removed in cleanup, if there is no
      * reference to them).
@@ -713,11 +720,7 @@ public class FileStore implements Segmen
         gcMonitor.info("TarMK compaction running, strategy={}", compactionStrategy);
 
         long start = System.currentTimeMillis();
-        SegmentWriter writer = new SegmentWriter(this, tracker, getVersion());
-        SegmentWriter mapWriter = compactionStrategy.getPersistCompactionMap()
-            ? new SegmentWriter(this, tracker, getVersion())
-            : null;
-        final Compactor compactor = new Compactor(writer, mapWriter, compactionStrategy.cloneBinaries());
+        Compactor compactor = new Compactor(this, compactionStrategy);
         SegmentNodeState before = getHead();
         long existing = before.getChildNode(SegmentNodeStore.CHECKPOINTS)
                 .getChildNodeCount(Long.MAX_VALUE);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/package-info.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/package-info.java Wed Jul 29 07:33:47 2015
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@Version("2.0.0")
+@Version("2.1.0")
 @Export(optional = "provide:=true")
 package org.apache.jackrabbit.oak.plugins.segment.file;
 

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/package-info.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/package-info.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/package-info.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/package-info.java Wed Jul 29 07:33:47 2015
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-@Version("3.1.0")
+@Version("4.0.0")
 @Export(optional = "provide:=true")
 package org.apache.jackrabbit.oak.plugins.segment;
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionAndCleanupTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionAndCleanupTest.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionAndCleanupTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionAndCleanupTest.java Wed Jul 29 07:33:47 2015
@@ -61,7 +61,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.Ignore;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -163,7 +162,7 @@ public class CompactionAndCleanupTest {
             // no data content =>
             // fileStore.size() == blobSize
             // some data content =>
-            // fileStore.size() in [blobSize + dataSize, blobSize + 2xdataSize]
+            // fileStore.size() in [blobSize + dataSize, blobSize + 2 x dataSize]
             assertTrue(fileStore.maybeCompact(false));
             fileStore.cleanup();
             assertSize("post cleanup", fileStore.size(), 0, blobSize + 2 * dataSize);
@@ -190,14 +189,12 @@ public class CompactionAndCleanupTest {
         }
     }
 
-    @Ignore("OAK-3139")  // FIXME OAK-3139
     @Test
     public void noCleanupOnCompactionMap() throws Exception {
         // 2MB data, 5MB blob
         final int blobSize = 5 * 1024 * 1024;
         final int dataNodes = 10000;
 
-        // really long time span, no binary cloning
         FileStore fileStore = new FileStore(directory, 1);
         final SegmentNodeStore nodeStore = new SegmentNodeStore(fileStore);
         CompactionStrategy custom = new CompactionStrategy(false, false,
@@ -231,33 +228,32 @@ public class CompactionAndCleanupTest {
             builder.setProperty("b", "foo");
             nodeStore.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
 
-            log.debug("File store pre removal {}, expecting {}",
-                    byteCountToDisplaySize(fileStore.size()),
-                    byteCountToDisplaySize(blobSize + dataSize));
-            assertEquals(mb(blobSize + dataSize), mb(fileStore.size()));
-
             // 2. Now remove the property
             builder = nodeStore.getRoot().builder();
             builder.removeProperty("a1");
             nodeStore.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
 
             // 3. Compact
-            assertTrue(fileStore.maybeCompact(false));
+            fileStore.maybeCompact(false);
 
             // 4. Add some more property to flush the current TarWriter
             builder = nodeStore.getRoot().builder();
             builder.setProperty("a2", createBlob(nodeStore, blobSize));
             nodeStore.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY);
 
+            // There should be no SNFE when running cleanup as compaction map segments
+            // should be pinned and thus not collected
+            fileStore.maybeCompact(false);
+            fileStore.cleanup();
+
             // refresh the ts ref, to simulate a long wait time
             custom.setOlderThan(0);
             TimeUnit.MILLISECONDS.sleep(5);
 
-            // This should not lead to a SNFE in the persisted compaction map (See OAK-3139)
             boolean needsCompaction = true;
             for (int i = 0; i < 3 && needsCompaction; i++) {
-                fileStore.cleanup();
                 needsCompaction = fileStore.maybeCompact(false);
+                fileStore.cleanup();
             }
         } finally {
             fileStore.close();
@@ -383,7 +379,7 @@ public class CompactionAndCleanupTest {
     }
 
     @Test
-    public void propertyRetention() throws IOException, CommitFailedException, InterruptedException {
+    public void propertyRetention() throws IOException, CommitFailedException {
         FileStore fileStore = new NonCachingFileStore(directory, 1);
         try {
             final SegmentNodeStore nodeStore = new SegmentNodeStore(fileStore);

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionMapTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionMapTest.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionMapTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/CompactionMapTest.java Wed Jul 29 07:33:47 2015
@@ -22,13 +22,15 @@ package org.apache.jackrabbit.oak.plugin
 import static com.google.common.collect.Iterables.get;
 import static com.google.common.collect.Maps.newHashMap;
 import static com.google.common.collect.Sets.newHashSet;
+import static java.io.File.createTempFile;
 import static org.apache.jackrabbit.oak.plugins.segment.CompactionMap.sum;
-import static org.apache.jackrabbit.oak.plugins.segment.SegmentVersion.V_11;
 import static org.apache.jackrabbit.oak.plugins.segment.TestUtils.randomRecordIdMap;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 
+import java.io.File;
+import java.io.IOException;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -36,16 +38,17 @@ import java.util.Random;
 import java.util.Set;
 import java.util.UUID;
 
+import javax.annotation.Nonnull;
+
 import com.google.common.collect.ImmutableList;
-import org.apache.jackrabbit.oak.plugins.segment.memory.MemoryStore;
+import org.apache.jackrabbit.oak.plugins.segment.file.FileStore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
 @RunWith(Parameterized.class)
 public class CompactionMapTest {
-    private final SegmentStore store = new MemoryStore();
-    private final SegmentTracker tracker = new SegmentTracker(store);
+    private final FileStore store;
     private final Random rnd = new Random();
 
     private final Map<RecordId, RecordId> referenceMap1;
@@ -63,30 +66,37 @@ public class CompactionMapTest {
         return ImmutableList.of(new Boolean[] {true}, new Boolean[] {false});
     }
 
-    private static PartialCompactionMap createCompactionMap(SegmentTracker tracker, SegmentWriter writer) {
-        if (writer != null) {
-            return new PersistedCompactionMap(writer);
+    private PartialCompactionMap createCompactionMap(boolean persisted) {
+        if (persisted) {
+            return new PersistedCompactionMap(store);
         } else {
-            return new InMemoryCompactionMap(tracker);
+            return new InMemoryCompactionMap(store.getTracker());
         }
     }
 
-    public CompactionMapTest(boolean usePersistedMap) {
-        SegmentWriter writer = usePersistedMap
-            ? new SegmentWriter(store, tracker, V_11)
-            : null;
-        compactionMap1 = createCompactionMap(tracker, writer);
-        referenceMap1 = randomRecordIdMap(rnd, tracker, 10, 10);
+    @Nonnull
+    private static File mkDir() throws IOException {
+        File directory = createTempFile(CompactionMapTest.class.getSimpleName(), "dir", new File("target"));
+        directory.delete();
+        directory.mkdir();
+        return directory;
+    }
+
+    public CompactionMapTest(boolean usePersistedMap) throws IOException {
+        store = FileStore.newFileStore(mkDir()).create();
+
+        compactionMap1 = createCompactionMap(usePersistedMap);
+        referenceMap1 = randomRecordIdMap(rnd, store.getTracker(), 10, 10);
         putAll(compactionMap1, referenceMap1);
         referenceMap.putAll(referenceMap1);
 
-        compactionMap2 = createCompactionMap(tracker, writer);
-        referenceMap2 = randomRecordIdMap(rnd, tracker, 10, 10);
+        compactionMap2 = createCompactionMap(usePersistedMap);
+        referenceMap2 = randomRecordIdMap(rnd, store.getTracker(), 10, 10);
         putAll(compactionMap2, referenceMap2);
         referenceMap.putAll(referenceMap2);
 
-        compactionMap3 = createCompactionMap(tracker, writer);
-        referenceMap3 = randomRecordIdMap(rnd, tracker, 10, 10);
+        compactionMap3 = createCompactionMap(usePersistedMap);
+        referenceMap3 = randomRecordIdMap(rnd, store.getTracker(), 10, 10);
         putAll(compactionMap3, referenceMap3);
         referenceMap.putAll(referenceMap3);
 
@@ -108,7 +118,7 @@ public class CompactionMapTest {
 
     @Test
     public void checkNonExistingKeys() {
-        for (RecordId keys : randomRecordIdMap(rnd, tracker, 10, 10).keySet()) {
+        for (RecordId keys : randomRecordIdMap(rnd, store.getTracker(), 10, 10).keySet()) {
             if (!referenceMap.containsKey(keys)) {
                 assertNull(compactionMap.get(keys));
             }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/PartialCompactionMapTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/PartialCompactionMapTest.java?rev=1693195&r1=1693194&r2=1693195&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/PartialCompactionMapTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/PartialCompactionMapTest.java Wed Jul 29 07:33:47 2015
@@ -51,6 +51,7 @@ import java.util.UUID;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.commons.benchmark.MicroBenchmark.Benchmark;
+import org.apache.jackrabbit.oak.plugins.segment.file.FileStore;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -66,7 +67,7 @@ public class PartialCompactionMapTest {
     private final boolean usePersistedMap;
 
     private File directory;
-    private SegmentStore segmentStore;
+    private FileStore segmentStore;
 
     private Map<RecordId, RecordId> reference;
     private PartialCompactionMap map;
@@ -102,7 +103,7 @@ public class PartialCompactionMapTest {
     private PartialCompactionMap createCompactionMap() {
         SegmentWriter writer = new SegmentWriter(segmentStore, getTracker(), V_11);
         if (usePersistedMap) {
-            return new PersistedCompactionMap(writer);
+            return new PersistedCompactionMap(segmentStore);
         } else {
             return new InMemoryCompactionMap(writer.getTracker());
         }