Author: mduerig
Date: Tue Dec 12 13:09:12 2017
New Revision: 1817912

URL: http://svn.apache.org/viewvc?rev=1817912&view=rev
Log:
OAK-7050: Offline compaction corrupts repository
Abort offline compaction without setting the file store head to the partially 
compacted head

Modified:
    
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreRestoreImpl.java
    
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/Compactor.java
    
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java
    
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactorTest.java

Modified: 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreRestoreImpl.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreRestoreImpl.java?rev=1817912&r1=1817911&r2=1817912&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreRestoreImpl.java
 (original)
+++ 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/backup/impl/FileStoreRestoreImpl.java
 Tue Dec 12 13:09:12 2017
@@ -85,7 +85,10 @@ public class FileStoreRestoreImpl implem
             );
             compactor.setContentEqualityCheck(true);
             SegmentNodeState after = compactor.compact(current, head, current);
-            store.getRevisions().setHead(current.getRecordId(), 
after.getRecordId());
+
+            if (after != null) {
+                store.getRevisions().setHead(current.getRecordId(), 
after.getRecordId());
+            }
         } finally {
             restore.close();
             store.close();

Modified: 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/Compactor.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/Compactor.java?rev=1817912&r1=1817911&r2=1817912&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/Compactor.java
 (original)
+++ 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/Compactor.java
 Tue Dec 12 13:09:12 2017
@@ -142,14 +142,6 @@ public class Compactor {
         this.binaryDedupMaxSize = gc.getBinaryDeduplicationMaxSize();
     }
 
-    private SegmentNodeBuilder process(NodeState before, NodeState after,
-            NodeState onto) throws IOException {
-        SegmentNodeBuilder builder = new SegmentNodeBuilder(
-                writer.writeNode(onto), writer);
-        new CompactDiff(builder).diff(before, after);
-        return builder;
-    }
-
     /**
      * Compact the differences between a {@code before} and a {@code after} on
      * top of an {@code onto} state.
@@ -160,16 +152,23 @@ public class Compactor {
      *            the after state
      * @param onto
      *            the onto state
-     * @return the compacted state
+     * @return the compacted state or {@code null} if compaction was cancelled
      */
     public SegmentNodeState compact(NodeState before, NodeState after,
             NodeState onto) throws IOException {
         progress.start();
-        SegmentNodeState compacted = process(before, after, onto)
-                .getNodeState();
-        writer.flush();
-        progress.stop();
-        return compacted;
+        try {
+            SegmentNodeBuilder builder = new 
SegmentNodeBuilder(writer.writeNode(onto), writer);
+            if (new CompactDiff(builder).diff(before, after)) {
+                SegmentNodeState compacted = builder.getNodeState();
+                writer.flush();
+                return compacted;
+            } else {
+                return null;
+            }
+        } finally {
+            progress.stop();
+        }
     }
 
     private class CompactDiff extends ApplyDiff {

Modified: 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java?rev=1817912&r1=1817911&r2=1817912&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java
 (original)
+++ 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java
 Tue Dec 12 13:09:12 2017
@@ -77,7 +77,6 @@ import org.apache.jackrabbit.oak.stats.C
 import org.apache.jackrabbit.oak.stats.DefaultStatisticsProvider;
 import org.apache.jackrabbit.oak.stats.StatisticsProvider;
 import org.junit.BeforeClass;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -281,7 +280,6 @@ public class CompactionAndCleanupIT {
     }
 
     @Test
-    @Ignore("OAK-7050")  // FIXME OAK-7050
     public void cancelOfflineCompaction() throws Exception {
         final AtomicBoolean cancelCompaction = new AtomicBoolean(true);
         try (FileStore fileStore = fileStoreBuilder(getFileStoreFolder())

Modified: 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactorTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactorTest.java?rev=1817912&r1=1817911&r2=1817912&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactorTest.java
 (original)
+++ 
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactorTest.java
 Tue Dec 12 13:09:12 2017
@@ -21,7 +21,7 @@ package org.apache.jackrabbit.oak.segmen
 import static 
org.apache.jackrabbit.oak.segment.SegmentWriterBuilder.segmentWriterBuilder;
 import static 
org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions.defaultGCOptions;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 
 import java.io.IOException;
 
@@ -69,18 +69,15 @@ public class CompactorTest {
 
     @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".
-
+        // Create a Compactor that will cancel itself as soon as possible. The 
early
+        // cancellation is the reason why the returned SegmentNodeState is null
         NodeStore store = 
SegmentNodeStoreBuilders.builder(memoryStore).build();
         SegmentWriter writer = 
segmentWriterBuilder("c").withGeneration(1).build(memoryStore);
         Compactor compactor = new Compactor(memoryStore.getReader(), writer,
                 memoryStore.getBlobStore(), Suppliers.ofInstance(true), 
defaultGCOptions());
-        SegmentNodeState sns = compactor.compact(store.getRoot(),
-                addChild(store.getRoot(), "b"), store.getRoot());
-        assertFalse(sns.hasChildNode("b"));
+        SegmentNodeState compacted = compactor.compact(
+                store.getRoot(), addChild(store.getRoot(), "b"), 
store.getRoot());
+        assertNull(compacted);
     }
 
     private NodeState addChild(NodeState current, String name) {


Reply via email to