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 2019/02/18 16:16:25 UTC
svn commit: r1853814 - in /jackrabbit/oak/branches/1.6: ./
oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentBufferWriterPool.java
oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java
Author: mduerig
Date: Mon Feb 18 16:16:25 2019
New Revision: 1853814
URL: http://svn.apache.org/viewvc?rev=1853814&view=rev
Log:
OAK-8033: Node states sometimes refer to more than a single generation of segments after a full compaction
Merged revision 1853429
Modified:
jackrabbit/oak/branches/1.6/ (props changed)
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentBufferWriterPool.java
jackrabbit/oak/branches/1.6/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CompactionAndCleanupIT.java
Propchange: jackrabbit/oak/branches/1.6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Feb 18 16:16:25 2019
@@ -1,4 +1,4 @@
/jackrabbit/oak/branches/1.0:1665962
/jackrabbit/oak/branches/1.8:1844835
-/jackrabbit/oak/trunk:1781068,1781075,1781248,1781386,1781846,1781907,1782000,1782029,1782196,1782447,1782476,1782770,1782945,1782966,1782973,1782990,1783061,1783066,1783089,1783104-1783105,1783110,1783619,1783720,1783731,1783733,1783738,1783742,1783773,1783855,1783891,1784023,1784034,1784130,1784162,1784251,1784401,1784551,1784574,1784689,1785095,1785108,1785161,1785172,1785283,1785652,1785838,1785916-1785917,1785919,1785946,1786122,1787074,1787145,1787151,1787217,1787425,1788056,1788378,1788387-1788389,1788463,1788476,1788850,1789056,1789534,1789925,1789940,1789987,1790006,1790013,1790069,1790077,1790079,1790382,1790502-1790503,1792049,1792463,1792742,1792746,1793013,1793088,1793618,1793627,1793644,1794393,1794417,1794683,1795138,1795314,1795330,1795475,1795488,1795491,1795502,1795594,1795613,1795618,1796144,1796230,1796239,1796274,1796278,1796988,1797378,1798035,1798832,1798834,1799219,1799389,1799393,1799924,1800244,1800269,1800606,1800613,1800974,1801011,1801013,1801118-1801119
,1801675,1802260,1802262,1802286,1802548,1802905,1802934,1802938,1802973,1803026,1803247-1803249,1803951,1803953-1803955,1804437,1805851-1805852,1806668,1807308,1807688,1808022,1808053,1808125,1808128,1808142,1808240,1808246,1808731,1809024,1809026,1809131,1809163,1809178-1809179,1809253,1809255-1809256,1809289,1809745,1811071-1811072,1811155,1811380,1811655,1811952,1811963,1811986,1813192,1813538,1814052,1814189,1814332,1814397,1814475,1815201,1815438,1815926,1816019,1817326,1817919,1817987-1817988,1817990,1818038,1818042,1818056,1818124,1818137,1818554,1818576,1818645,1819048,1819050,1821237,1821325,1821358,1821495,1821516,1821847,1822207,1822850,1823172,1823655,1824896,1825471,1825654,1826237,1826338,1826532,1826640,1826932,1826957,1827472,1827486,1827977,1828502,1829527,1829569,1829587,1829665,1829854,1829864,1829987,1829998,1830019,1830160,1830239,1830748,1831190,1831374,1832379,1832535,1833308,1834648-1834649,1834681,1835060,1835780,1836082,1837475,1837998,1838637,1839746,1840
024,1840226,1842677,1843175,1843222,1843231,1844549,1844642,1844728,1845135,1845405,1845415,1845730-1845731,1845863,1845865,1846057,1846617,1848181-1848182,1848191,1848217,1848822-1848823,1850837
+/jackrabbit/oak/trunk:1781068,1781075,1781248,1781386,1781846,1781907,1782000,1782029,1782196,1782447,1782476,1782770,1782945,1782966,1782973,1782990,1783061,1783066,1783089,1783104-1783105,1783110,1783619,1783720,1783731,1783733,1783738,1783742,1783773,1783855,1783891,1784023,1784034,1784130,1784162,1784251,1784401,1784551,1784574,1784689,1785095,1785108,1785161,1785172,1785283,1785652,1785838,1785916-1785917,1785919,1785946,1786122,1787074,1787145,1787151,1787217,1787425,1788056,1788378,1788387-1788389,1788463,1788476,1788850,1789056,1789534,1789925,1789940,1789987,1790006,1790013,1790069,1790077,1790079,1790382,1790502-1790503,1792049,1792463,1792742,1792746,1793013,1793088,1793618,1793627,1793644,1794393,1794417,1794683,1795138,1795314,1795330,1795475,1795488,1795491,1795502,1795594,1795613,1795618,1796144,1796230,1796239,1796274,1796278,1796988,1797378,1798035,1798832,1798834,1799219,1799389,1799393,1799924,1800244,1800269,1800606,1800613,1800974,1801011,1801013,1801118-1801119
,1801675,1802260,1802262,1802286,1802548,1802905,1802934,1802938,1802973,1803026,1803247-1803249,1803951,1803953-1803955,1804437,1805851-1805852,1806668,1807308,1807688,1808022,1808053,1808125,1808128,1808142,1808240,1808246,1808731,1809024,1809026,1809131,1809163,1809178-1809179,1809253,1809255-1809256,1809289,1809745,1811071-1811072,1811155,1811380,1811655,1811952,1811963,1811986,1813192,1813538,1814052,1814189,1814332,1814397,1814475,1815201,1815438,1815926,1816019,1817326,1817919,1817987-1817988,1817990,1818038,1818042,1818056,1818124,1818137,1818554,1818576,1818645,1819048,1819050,1821237,1821325,1821358,1821495,1821516,1821847,1822207,1822850,1823172,1823655,1824896,1825471,1825654,1826237,1826338,1826532,1826640,1826932,1826957,1827472,1827486,1827977,1828502,1829527,1829569,1829587,1829665,1829854,1829864,1829987,1829998,1830019,1830160,1830239,1830748,1831190,1831374,1832379,1832535,1833308,1834648-1834649,1834681,1835060,1835780,1836082,1837475,1837998,1838637,1839746,1840
024,1840226,1842677,1843175,1843222,1843231,1844549,1844642,1844728,1845135,1845405,1845415,1845730-1845731,1845863,1845865,1846057,1846617,1848181-1848182,1848191,1848217,1848822-1848823,1850837,1853429
/jackrabbit/trunk:1345480
Modified: jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentBufferWriterPool.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentBufferWriterPool.java?rev=1853814&r1=1853813&r2=1853814&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentBufferWriterPool.java (original)
+++ jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentBufferWriterPool.java Mon Feb 18 16:16:25 2019
@@ -115,7 +115,7 @@ public class SegmentBufferWriterPool imp
@Nonnull WriteOperation writeOperation)
throws IOException {
SimpleImmutableEntry<?,?> key = new SimpleImmutableEntry<>(currentThread(), generation);
- SegmentBufferWriter writer = borrowWriter(key);
+ SegmentBufferWriter writer = borrowWriter(key, generation);
try {
return writeOperation.execute(writer);
} finally {
@@ -204,7 +204,7 @@ public class SegmentBufferWriterPool imp
* a fresh writer at any time. Callers need to return a writer before
* borrowing it again. Failing to do so leads to undefined behaviour.
*/
- private SegmentBufferWriter borrowWriter(Object key) {
+ private SegmentBufferWriter borrowWriter(@Nonnull Object key, int generation) {
poolMonitor.enter();
try {
SegmentBufferWriter writer = writers.remove(key);
@@ -214,7 +214,7 @@ public class SegmentBufferWriterPool imp
tracker.getSegmentCounter(),
reader,
getWriterId(wid),
- gcGeneration.get()
+ generation
);
}
borrowed.add(writer);
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=1853814&r1=1853813&r2=1853814&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 Mon Feb 18 16:16:25 2019
@@ -20,6 +20,7 @@
package org.apache.jackrabbit.oak.segment;
import static com.google.common.collect.Lists.newArrayList;
+import static com.google.common.util.concurrent.Uninterruptibles.awaitUninterruptibly;
import static com.google.common.util.concurrent.Uninterruptibles.sleepUninterruptibly;
import static java.lang.Integer.getInteger;
import static java.lang.String.valueOf;
@@ -48,6 +49,7 @@ import java.util.Random;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
@@ -59,11 +61,11 @@ import java.util.concurrent.atomic.Atomi
import com.google.common.io.ByteStreams;
import org.apache.jackrabbit.oak.api.Blob;
-import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
import org.apache.jackrabbit.oak.plugins.blob.ReferenceCollector;
+import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState;
import org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions;
import org.apache.jackrabbit.oak.segment.file.FileStore;
import org.apache.jackrabbit.oak.segment.file.FileStoreGCMonitor;
@@ -661,17 +663,24 @@ public class CompactionAndCleanupIT {
.withMemoryMapping(true)
.build();
final SegmentNodeStore nodeStore = SegmentNodeStoreBuilders.builder(store).build();
- final AtomicBoolean compactionSuccess = new AtomicBoolean(true);
+ AtomicBoolean compactionSuccess = new AtomicBoolean(true);
NodeBuilder root = nodeStore.getRoot().builder();
createNodes(root.setChildNode("test"), 10, 3);
nodeStore.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
final Set<UUID> beforeSegments = new HashSet<UUID>();
- collectSegments(store.getReader(), store.getRevisions(), beforeSegments);
+ collectSegments(store.getReader(), store.getHead().getRecordId(),
+ new Consumer<SegmentId>() {
+ @Override
+ public void accept(SegmentId segmentId) {
+ beforeSegments.add(segmentId.asUUID());
+ }
+ });
+
final AtomicReference<Boolean> run = new AtomicReference<Boolean>(true);
- final List<String> failedCommits = newArrayList();
+ final List<Exception> failedCommits = newArrayList();
Thread[] threads = new Thread[10];
for (int k = 0; k < threads.length; k++) {
final int threadId = k;
@@ -681,15 +690,15 @@ public class CompactionAndCleanupIT {
for (int j = 0; run.get(); j++) {
String nodeName = "b-" + threadId + "," + j;
try {
- NodeBuilder root = nodeStore.getRoot().builder();
- root.setChildNode(nodeName);
- nodeStore.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ NodeBuilder changes = nodeStore.getRoot().builder();
+ changes.setChildNode(nodeName);
+ nodeStore.merge(changes, EmptyHook.INSTANCE, CommitInfo.EMPTY);
Thread.sleep(5);
- } catch (CommitFailedException e) {
- failedCommits.add(nodeName);
} catch (InterruptedException e) {
Thread.interrupted();
break;
+ } catch (Exception e) {
+ failedCommits.add(new ExecutionException("Failed commit " + nodeName, e));
}
}
}
@@ -704,10 +713,18 @@ public class CompactionAndCleanupIT {
store.flush();
assumeTrue("Failed to acquire compaction lock", compactionSuccess.get());
- assertTrue("Failed commits: " + failedCommits, failedCommits.isEmpty());
+ for (Exception failedCommit : failedCommits) {
+ throw new Exception("A background commit failed", failedCommit);
+ }
- Set<UUID> afterSegments = new HashSet<UUID>();
- collectSegments(store.getReader(), store.getRevisions(), afterSegments);
+ final Set<UUID> afterSegments = new HashSet<UUID>();
+ collectSegments(store.getReader(), store.getHead().getRecordId(),
+ new Consumer<SegmentId>() {
+ @Override
+ public void accept(SegmentId segmentId) {
+ afterSegments.add(segmentId.asUUID());
+ }
+ });
try {
for (UUID u : beforeSegments) {
assertFalse("Mixed segments found: " + u, afterSegments.contains(u));
@@ -717,6 +734,66 @@ public class CompactionAndCleanupIT {
}
}
+ @Test
+ public void testMixedSegmentsGCGeneration() throws Exception {
+ try (FileStore store = fileStoreBuilder(getFileStoreFolder())
+ .withMaxFileSize(2)
+ .withMemoryMapping(true)
+ .build()) {
+
+ final CountDownLatch readyToCompact = new CountDownLatch(1);
+ final CountDownLatch compactionCompleted = new CountDownLatch(1);
+ final SegmentNodeBuilder changes = store.getHead().builder();
+ changes.setProperty("a", "a");
+ changes.setProperty(new StringPropertyState("b", "b") {
+ @Override
+ public String getValue() {
+ readyToCompact.countDown();
+ awaitUninterruptibly(compactionCompleted);
+ return super.getValue();
+ }
+ });
+
+ // Overlap an ongoing write operation triggered by the call to getNodeState
+ // with a full compaction. This should not cause the written node state
+ // to reference segments from multiple generations
+ FutureTask<SegmentNodeState> futureNodeState = runAsync(new Callable<SegmentNodeState>() {
+ @Override
+ public SegmentNodeState call() throws Exception {
+ return changes.getNodeState();
+ }
+ });
+ readyToCompact.await();
+
+ store.compact();
+ compactionCompleted.countDown();
+
+ // The node state from the write operation that started before
+ // compaction completed should reference only segments from generation 0.
+ SegmentNodeState gen0NodeState = futureNodeState.get();
+ collectSegments(store.getReader(), gen0NodeState.getRecordId(),
+ new Consumer<SegmentId>() {
+ @Override
+ public void accept(SegmentId segmentId) {
+ assertEquals("Full generation should be 0",
+ 0, segmentId.getGcGeneration());
+ }
+ });
+
+ // Retrieving the node state again should trigger a rewriting to
+ // the next generation (1).
+ SegmentNodeState gen1NodeState = changes.getNodeState();
+ collectSegments(store.getReader(), gen1NodeState.getRecordId(),
+ new Consumer<SegmentId>() {
+ @Override
+ public void accept(SegmentId segmentId) {
+ assertEquals("Full generation should be 1",
+ 1, segmentId.getGcGeneration());
+ }
+ });
+ }
+ }
+
/**
* Set a root node referring to a child node that lives in a different segments. Depending
* on the order how the SegmentBufferWriters associated with the threads used to create the
@@ -849,81 +926,84 @@ public class CompactionAndCleanupIT {
}
}
- private static void collectSegments(SegmentReader reader, Revisions revisions,
- final Set<UUID> segmentIds) {
+ private interface Consumer<T> {
+ void accept(T value);
+ }
+
+ private static void collectSegments(SegmentReader reader, RecordId headId, final Consumer<SegmentId> onSegment) {
new SegmentParser(reader) {
@Override
protected void onNode(RecordId parentId, RecordId nodeId) {
super.onNode(parentId, nodeId);
- segmentIds.add(nodeId.asUUID());
+ onSegment.accept(nodeId.getSegmentId());
}
@Override
protected void onTemplate(RecordId parentId, RecordId templateId) {
super.onTemplate(parentId, templateId);
- segmentIds.add(templateId.asUUID());
+ onSegment.accept(templateId.getSegmentId());
}
@Override
protected void onMap(RecordId parentId, RecordId mapId, MapRecord map) {
super.onMap(parentId, mapId, map);
- segmentIds.add(mapId.asUUID());
+ onSegment.accept(mapId.getSegmentId());
}
@Override
protected void onMapDiff(RecordId parentId, RecordId mapId, MapRecord map) {
super.onMapDiff(parentId, mapId, map);
- segmentIds.add(mapId.asUUID());
+ onSegment.accept(mapId.getSegmentId());
}
@Override
protected void onMapLeaf(RecordId parentId, RecordId mapId, MapRecord map) {
super.onMapLeaf(parentId, mapId, map);
- segmentIds.add(mapId.asUUID());
+ onSegment.accept(mapId.getSegmentId());
}
@Override
protected void onMapBranch(RecordId parentId, RecordId mapId, MapRecord map) {
super.onMapBranch(parentId, mapId, map);
- segmentIds.add(mapId.asUUID());
+ onSegment.accept(mapId.getSegmentId());
}
@Override
protected void onProperty(RecordId parentId, RecordId propertyId, PropertyTemplate template) {
super.onProperty(parentId, propertyId, template);
- segmentIds.add(propertyId.asUUID());
+ onSegment.accept(propertyId.getSegmentId());
}
@Override
protected void onValue(RecordId parentId, RecordId valueId, Type<?> type) {
super.onValue(parentId, valueId, type);
- segmentIds.add(valueId.asUUID());
+ onSegment.accept(valueId.getSegmentId());
}
@Override
protected void onBlob(RecordId parentId, RecordId blobId) {
super.onBlob(parentId, blobId);
- segmentIds.add(blobId.asUUID());
+ onSegment.accept(blobId.getSegmentId());
}
@Override
protected void onString(RecordId parentId, RecordId stringId) {
super.onString(parentId, stringId);
- segmentIds.add(stringId.asUUID());
+ onSegment.accept(stringId.getSegmentId());
}
@Override
protected void onList(RecordId parentId, RecordId listId, int count) {
super.onList(parentId, listId, count);
- segmentIds.add(listId.asUUID());
+ onSegment.accept(listId.getSegmentId());
}
@Override
protected void onListBucket(RecordId parentId, RecordId listId, int index, int count, int capacity) {
super.onListBucket(parentId, listId, index, count, capacity);
- segmentIds.add(listId.asUUID());
+ onSegment.accept(listId.getSegmentId());
}
- }.parseNode(revisions.getHead());
+ }.parseNode(headId);
}
private static void createNodes(NodeBuilder builder, int count, int depth) {
@@ -1148,13 +1228,13 @@ public class CompactionAndCleanupIT {
ExecutorService executorService = newFixedThreadPool(300);
final AtomicInteger counter = new AtomicInteger();
final ReferenceCollector dummyCollector = new ReferenceCollector() {
-
+
@Override
public void addReference(String reference, String nodeId) {
// do nothing
}
};
-
+
try {
Callable<Void> concurrentWriteTask = new Callable<Void>() {
@Override