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.