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 fr...@apache.org on 2018/09/20 08:39:43 UTC
svn commit: r1841442 - in
/jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file:
AbstractFileStore.java FileStore.java
Author: frm
Date: Thu Sep 20 08:39:43 2018
New Revision: 1841442
URL: http://svn.apache.org/viewvc?rev=1841442&view=rev
Log:
OAK-7755 - Fix deadlock in FileStore#writeSegment
When writing a segment, the FileStore first acquire the fileStoreLock in write
mode, then locks the segment to read its binary references. When reading a
segment, the FileStore first locks the segment and then acquire the
fileStoreLock in read mode. In order to break this deadlock, this commit fixes
the write operation so that the segment references are read without acquiring
the fileStoreLock first.
Modified:
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/AbstractFileStore.java
jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java
Modified: jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/AbstractFileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/AbstractFileStore.java?rev=1841442&r1=1841441&r2=1841442&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/AbstractFileStore.java (original)
+++ jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/AbstractFileStore.java Thu Sep 20 08:39:43 2018
@@ -28,7 +28,9 @@ import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.regex.Matcher;
@@ -37,6 +39,7 @@ import java.util.regex.Pattern;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
+import com.google.common.base.Supplier;
import org.apache.jackrabbit.oak.api.jmx.CacheStatsMBean;
import org.apache.jackrabbit.oak.segment.CachingSegmentReader;
import org.apache.jackrabbit.oak.segment.RecordType;
@@ -56,8 +59,6 @@ import org.apache.jackrabbit.oak.spi.blo
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.google.common.base.Supplier;
-
/**
* The storage implementation for tar files.
*/
@@ -331,31 +332,47 @@ public abstract class AbstractFileStore
w.writeEntry(msb, lsb, data, 0, data.length, generation);
if (SegmentId.isDataSegmentId(lsb)) {
Segment segment = new Segment(this, segmentReader, newSegmentId(msb, lsb), buffer);
- populateTarGraph(segment, w);
- populateTarBinaryReferences(segment, w);
+ populateTarGraph(segment, w, readGraphReferences(segment));
+ populateTarBinaryReferences(segment, w, readBinaryReferences(segment));
}
}
- static void populateTarGraph(Segment segment, TarWriter w) {
+ static void populateTarGraph(Segment segment, TarWriter w, Iterable<UUID> references) {
UUID from = segment.getSegmentId().asUUID();
+ for (UUID reference : references) {
+ w.addGraphEdge(from, reference);
+ }
+ }
+
+ static Iterable<UUID> readGraphReferences(Segment segment) {
+ List<UUID> reference = new ArrayList<>();
for (int i = 0; i < segment.getReferencedSegmentIdCount(); i++) {
- w.addGraphEdge(from, segment.getReferencedSegmentId(i));
+ reference.add(segment.getReferencedSegmentId(i));
+ }
+ return reference;
+ }
+
+ static void populateTarBinaryReferences(Segment segment, TarWriter w, Iterable<String> references) {
+ int generation = segment.getGcGeneration();
+ UUID id = segment.getSegmentId().asUUID();
+ for (String reference : references) {
+ w.addBinaryReference(generation, id, reference);
}
}
- static void populateTarBinaryReferences(final Segment segment, final TarWriter w) {
- final int generation = segment.getGcGeneration();
- final UUID id = segment.getSegmentId().asUUID();
+ static Iterable<String> readBinaryReferences(final Segment segment) {
+ final List<String> references = new ArrayList<>();
segment.forEachRecord(new RecordConsumer() {
@Override
public void consume(int number, RecordType type, int offset) {
if (type == RecordType.BLOB_ID) {
- w.addBinaryReference(generation, id, SegmentBlob.readBlobId(segment, number));
+ references.add(SegmentBlob.readBlobId(segment, number));
}
}
});
+ return references;
}
static void closeAndLogOnFail(Closeable closeable) {
Modified: jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java?rev=1841442&r1=1841441&r2=1841442&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java (original)
+++ jackrabbit/oak/branches/1.6/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/file/FileStore.java Thu Sep 20 08:39:43 2018
@@ -615,6 +615,8 @@ public class FileStore extends AbstractF
@Override
public void writeSegment(SegmentId id, byte[] buffer, int offset, int length) throws IOException {
Segment segment = null;
+ Iterable<UUID> graphReferences = null;
+ Iterable<String> binaryReferences = null;
// If the segment is a data segment, create a new instance of Segment to
// access some internal information stored in the segment and to store
@@ -634,6 +636,8 @@ public class FileStore extends AbstractF
segment = new Segment(this, segmentReader, id, data);
generation = segment.getGcGeneration();
+ graphReferences = readGraphReferences(segment);
+ binaryReferences = readBinaryReferences(segment);
}
fileStoreLock.writeLock().lock();
@@ -653,8 +657,8 @@ public class FileStore extends AbstractF
// (potentially) flushing the TAR file.
if (segment != null) {
- populateTarGraph(segment, tarWriter);
- populateTarBinaryReferences(segment, tarWriter);
+ populateTarGraph(segment, tarWriter, graphReferences);
+ populateTarBinaryReferences(segment, tarWriter, binaryReferences);
}
// Close the TAR file if the size exceeds the maximum.