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) {