Author: mduerig
Date: Fri Jul 17 14:05:08 2015
New Revision: 1691566

URL: http://svn.apache.org/r1691566
Log:
OAK-3107: SegmentWriter should be able to store blob IDs longer than 4096 bytes 
Merged revision 1691509

Modified:
    jackrabbit/oak/branches/1.2/   (props changed)
    
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
    
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentBlob.java
    
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java
    
jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/ExternalBlobReferenceTest.java

Propchange: jackrabbit/oak/branches/1.2/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Jul 17 14:05:08 2015
@@ -1,3 +1,3 @@
 /jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1672350,1672468,1672537,1672603,1672642,1672644,1672834-1672835,1673351,1673410,1673414-1673415,1673436,1673644,1673662-1673664,1673669,1673695,1673738,1673787,1673791,1674046,1674065,1674075,1674107,1674228,1674780,1674880,1675054-1675055,1675319,1675332,1675354,1675357,1675382,1675555,1675566,1675593,1676198,1676237,1676407,1676458,1676539,1676670,1676693,1676703,1676725,1677579,1677581,1677609,1677611,1677774,1677788,1677797,1677804,1677806,1677939,1677991,1678023,1678095-1678096,1678171,1678173,1678211,1678323,1678758,1678938,1678954,1679144,1679165,1679191,1679232,1679235,1679503,1679958,1679961,1680170,1680182,1680222,1680232,1680236,1680461,1680633,1680643,1680747,1680805-1680806,1680903,1681282,1681767,1681918,1682042,1682218,1682235,1682437,1682494,1682555,1682855,1682904,1683059,1683089,1683213,1683249,1683259,1683278,1683323,1683687,1683700,1684174-1684175,1684186,1684376,1684442,1684561,1684570,1684601,1684618,1684820,1684868,1685023,1685075,1685370
 
,1685552,1685589-1685590,1685840,1685964,1685977,1685989,1685999,1686023,1686032,1686097,1686162,1686229,1686234,1686253,1686414,1686780,1686854,1686857,1686971,1687053-1687055,1687175,1687196,1687198,1687220,1687239-1687240,1687301,1687441,1687553,1688089-1688090,1688172,1688179,1688349,1688421,1688436,1688453,1688616,1688622,1688634,1688636,1688817,1689003-1689004,1689008,1689577,1689581,1689623,1689810,1689828,1689833,1689903,1690017,1690043,1690047,1690057,1690247,1690249,1690634-1690637,1690650,1690669,1690674,1690885,1690941,1691151,1691167,1691183,1691210,1691307,1691331-1691333,1691345,1691384-1691385,1691401
+/jackrabbit/oak/trunk:1672350,1672468,1672537,1672603,1672642,1672644,1672834-1672835,1673351,1673410,1673414-1673415,1673436,1673644,1673662-1673664,1673669,1673695,1673738,1673787,1673791,1674046,1674065,1674075,1674107,1674228,1674780,1674880,1675054-1675055,1675319,1675332,1675354,1675357,1675382,1675555,1675566,1675593,1676198,1676237,1676407,1676458,1676539,1676670,1676693,1676703,1676725,1677579,1677581,1677609,1677611,1677774,1677788,1677797,1677804,1677806,1677939,1677991,1678023,1678095-1678096,1678171,1678173,1678211,1678323,1678758,1678938,1678954,1679144,1679165,1679191,1679232,1679235,1679503,1679958,1679961,1680170,1680182,1680222,1680232,1680236,1680461,1680633,1680643,1680747,1680805-1680806,1680903,1681282,1681767,1681918,1682042,1682218,1682235,1682437,1682494,1682555,1682855,1682904,1683059,1683089,1683213,1683249,1683259,1683278,1683323,1683687,1683700,1684174-1684175,1684186,1684376,1684442,1684561,1684570,1684601,1684618,1684820,1684868,1685023,1685075,1685370
 
,1685552,1685589-1685590,1685840,1685964,1685977,1685989,1685999,1686023,1686032,1686097,1686162,1686229,1686234,1686253,1686414,1686780,1686854,1686857,1686971,1687053-1687055,1687175,1687196,1687198,1687220,1687239-1687240,1687301,1687441,1687553,1688089-1688090,1688172,1688179,1688349,1688421,1688436,1688453,1688616,1688622,1688634,1688636,1688817,1689003-1689004,1689008,1689577,1689581,1689623,1689810,1689828,1689833,1689903,1690017,1690043,1690047,1690057,1690247,1690249,1690634-1690637,1690650,1690669,1690674,1690885,1690941,1691151,1691167,1691183,1691210,1691307,1691331-1691333,1691345,1691384-1691385,1691401,1691509
 /jackrabbit/trunk:1345480

Modified: 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java?rev=1691566&r1=1691565&r2=1691566&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
 (original)
+++ 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java
 Fri Jul 17 14:05:08 2015
@@ -97,6 +97,15 @@ public class Segment {
      */
     public static final int MEDIUM_LIMIT = (1 << (16 - 2)) + SMALL_LIMIT;
 
+    /**
+     * Maximum size of small blob IDs. A small blob ID is stored in a value
+     * record whose length field contains the pattern "1110" in its most
+     * significant bits. Since two bytes are used to store both the bit pattern
+     * and the actual length of the blob ID, a maximum of 2^12 values can be
+     * stored in the length field.
+     */
+    public static final int BLOB_ID_SMALL_LIMIT = 1 << 12;
+
     public static int REF_COUNT_OFFSET = 5;
 
     static int ROOT_COUNT_OFFSET = 6;

Modified: 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentBlob.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentBlob.java?rev=1691566&r1=1691565&r2=1691566&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentBlob.java
 (original)
+++ 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentBlob.java
 Fri Jul 17 14:05:08 2015
@@ -79,10 +79,11 @@ public class SegmentBlob extends Record
                     segment.readRecordId(offset + 8), listSize);
             return new SegmentStream(getRecordId(), list, length);
         } else if ((head & 0xf0) == 0xe0) {
-            // 1110 xxxx: external value
-            String refererence = readReference(segment, offset, head);
-            return segment.getSegmentId().getTracker().getStore()
-                    .readBlob(refererence).getNewStream();
+            // 1110 xxxx: external value, short blob ID
+            return getNewStream(readShortBlobId(segment, offset, head));
+        } else if ((head & 0xf8) == 0xf0) {
+            // 1111 0xxx: external value, long blob ID
+            return getNewStream(readLongBlobId(segment, offset));
         } else {
             throw new IllegalStateException(String.format(
                     "Unexpected value record type: %02x", head & 0xff));
@@ -104,15 +105,11 @@ public class SegmentBlob extends Record
             // 110x xxxx: long value
             return (segment.readLong(offset) & 0x1fffffffffffffffL) + 
MEDIUM_LIMIT;
         } else if ((head & 0xf0) == 0xe0) {
-            // 1110 xxxx: external value
-            String reference = readReference(segment, offset, head);
-            long length = segment.getSegmentId().getTracker().getStore()
-                    .readBlob(reference).length();
-            if (length == -1) {
-                throw new IllegalStateException(
-                        "Unknown length of external binary: " + reference);
-            }
-            return length;
+            // 1110 xxxx: external value, short blob ID
+            return getLength(readShortBlobId(segment, offset, head));
+        } else if ((head & 0xf8) == 0xf0) {
+            // 1111 0xxx: external value, long blob ID
+            return getLength(readLongBlobId(segment, offset));
         } else {
             throw new IllegalStateException(String.format(
                     "Unexpected value record type: %02x", head & 0xff));
@@ -150,8 +147,8 @@ public class SegmentBlob extends Record
         Segment segment = getSegment();
         int offset = getOffset();
         byte head = segment.readByte(offset);
-        // 1110 xxxx: external value
-        return (head & 0xf0) == 0xe0;
+        // 1110 xxxx or 1111 0xxx: external value
+        return (head & 0xf0) == 0xe0 || (head & 0xf8) == 0xf0;
     }
 
     public String getBlobId() {
@@ -159,8 +156,11 @@ public class SegmentBlob extends Record
         int offset = getOffset();
         byte head = segment.readByte(offset);
         if ((head & 0xf0) == 0xe0) {
-            // 1110 xxxx: external value
-            return readReference(segment, offset, head);
+            // 1110 xxxx: external value, small blob ID
+            return readShortBlobId(segment, offset, head);
+        } else if ((head & 0xf8) == 0xf0) {
+            // 1111 0xxx: external value, long blob ID
+            return readLongBlobId(segment, offset);
         } else {
             return null;
         }
@@ -191,7 +191,10 @@ public class SegmentBlob extends Record
                 return writer.writeLargeBlob(length, list.getEntries());
             }
         } else if ((head & 0xf0) == 0xe0) {
-            // 1110 xxxx: external value
+            // 1110 xxxx: external value, short blob ID
+            return writer.writeExternalBlob(getBlobId());
+        } else if ((head & 0xf8) == 0xf0) {
+            // 1111 0xxx: external value, long blob ID
             return writer.writeExternalBlob(getBlobId());
         } else {
             throw new IllegalStateException(String.format(
@@ -222,14 +225,18 @@ public class SegmentBlob extends Record
 
     //-----------------------------------------------------------< private >--
 
-    private static String readReference(
-            Segment segment, int offset, byte head) {
+    private static String readShortBlobId(Segment segment, int offset, byte 
head) {
         int length = (head & 0x0f) << 8 | (segment.readByte(offset + 1) & 
0xff);
         byte[] bytes = new byte[length];
         segment.readBytes(offset + 2, bytes, 0, length);
         return new String(bytes, UTF_8);
     }
 
+    private static String readLongBlobId(Segment segment, int offset) {
+        RecordId blobIdRecordId = segment.readRecordId(offset + 1);
+        return segment.readString(blobIdRecordId);
+    }
+
     private Iterable<SegmentId> getBulkSegmentIds() {
         Segment segment = getSegment();
         int offset = getOffset();
@@ -250,4 +257,22 @@ public class SegmentBlob extends Record
         }
     }
 
+    private Blob getBlob(String blobId) {
+        return 
getSegment().getSegmentId().getTracker().getStore().readBlob(blobId);
+    }
+
+    private InputStream getNewStream(String blobId) {
+        return getBlob(blobId).getNewStream();
+    }
+
+    private long getLength(String blobId) {
+        long length = getBlob(blobId).length();
+
+        if (length == -1) {
+            throw new IllegalStateException(String.format("Unknown length of 
external binary: %s", blobId));
+        }
+
+        return length;
+    }
+
 }

Modified: 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java?rev=1691566&r1=1691565&r2=1691566&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java
 (original)
+++ 
jackrabbit/oak/branches/1.2/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java
 Fri Jul 17 14:05:08 2015
@@ -586,32 +586,77 @@ public class SegmentWriter {
     }
 
     /**
-     * Write a reference to an external blob.
+     * Write a reference to an external blob. This method handles blob IDs of
+     * every length, but behaves differently for small and large blob IDs.
      *
-     * @param reference reference
-     * @return record id
+     * @param blobId Blob ID.
+     * @return Record ID pointing to the written blob ID.
+     * @see Segment#BLOB_ID_SMALL_LIMIT
      */
-    private synchronized RecordId writeValueRecord(String reference) {
-        byte[] data = reference.getBytes(Charsets.UTF_8);
-        int length = data.length;
-
-        // When writing a binary ID, the four most significant bits of the
-        // length field should be "1110", leaving 12 other bits to store the
-        // length itself. This means that the values of the length field can
-        // only range between 0 and 2^12 - 1.
-
-        checkArgument(length < 4096);
-
-        RecordId id = prepare(RecordType.VALUE, 2 + length);
-        int len = length | 0xE000;
-        buffer[position++] = (byte) (len >> 8);
-        buffer[position++] = (byte) len;
+    private RecordId writeBlobId(String blobId) {
+        byte[] data = blobId.getBytes(Charsets.UTF_8);
 
-        System.arraycopy(data, 0, buffer, position, length);
-        position += length;
+        if (data.length < Segment.BLOB_ID_SMALL_LIMIT) {
+            return writeSmallBlobId(data);
+        } else {
+            return writeLargeBlobId(blobId);
+        }
+    }
+
+    /**
+     * Write a large blob ID. A blob ID is considered large if the length of 
its
+     * binary representation is equal to or greater than {@code
+     * Segment.BLOB_ID_SMALL_LIMIT}.
+     *
+     * @param blobId Blob ID.
+     * @return A record ID pointing to the written blob ID.
+     */
+    private RecordId writeLargeBlobId(String blobId) {
+        RecordId stringRecord = writeString(blobId);
+
+        synchronized (this) {
+            RecordId blobIdRecord = prepare(RecordType.VALUE, 1, 
Collections.singletonList(stringRecord));
+
+            // The length uses a fake "length" field that is always equal to 
0xF0.
+            // This allows the code to take apart small from a large blob IDs.
+
+            buffer[position++] = (byte) 0xF0;
+            writeRecordId(stringRecord);
+
+            blobrefs.add(blobIdRecord);
+
+            return blobIdRecord;
+        }
+    }
+
+    /**
+     * Write a small blob ID. A blob ID is considered small if the length of 
its
+     * binary representation is less than {@code Segment.BLOB_ID_SMALL_LIMIT}.
+     *
+     * @param blobId Blob ID.
+     * @return A record ID pointing to the written blob ID.
+     */
+    private RecordId writeSmallBlobId(byte[] blobId) {
+        int length = blobId.length;
+
+        checkArgument(length < Segment.BLOB_ID_SMALL_LIMIT);
+
+        synchronized (this) {
+            RecordId id = prepare(RecordType.VALUE, 2 + length);
+
+            int masked = length | 0xE000;
+
+            buffer[position++] = (byte) (masked >> 8);
+            buffer[position++] = (byte) (masked);
+
+            System.arraycopy(blobId, 0, buffer, position, length);
 
-        blobrefs.add(id);
-        return id;
+            position += length;
+
+            blobrefs.add(id);
+
+            return id;
+        }
     }
 
     /**
@@ -778,7 +823,7 @@ public class SegmentWriter {
         if (reference != null && store.getBlobStore() != null) {
             String blobId = store.getBlobStore().getBlobId(reference);
             if (blobId != null) {
-                RecordId id = writeValueRecord(blobId);
+                RecordId id = writeBlobId(blobId);
                 return new SegmentBlob(id);
             } else {
                 log.debug("No blob found for reference {}, inlining...", 
reference);
@@ -788,8 +833,8 @@ public class SegmentWriter {
         return writeStream(blob.getNewStream());
     }
 
-    SegmentBlob writeExternalBlob(String blobId) throws IOException {
-        RecordId id = writeValueRecord(blobId);
+    SegmentBlob writeExternalBlob(String blobId) {
+        RecordId id = writeBlobId(blobId);
         return new SegmentBlob(id);
     }
 
@@ -837,7 +882,7 @@ public class SegmentWriter {
         } else if (blobStore != null) {
             String blobId = blobStore.writeBlob(new SequenceInputStream(
                     new ByteArrayInputStream(data, 0, n), stream));
-            return writeValueRecord(blobId);
+            return writeBlobId(blobId);
         }
 
         long length = n;

Modified: 
jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/ExternalBlobReferenceTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/ExternalBlobReferenceTest.java?rev=1691566&r1=1691565&r2=1691566&view=diff
==============================================================================
--- 
jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/ExternalBlobReferenceTest.java
 (original)
+++ 
jackrabbit/oak/branches/1.2/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/ExternalBlobReferenceTest.java
 Fri Jul 17 14:05:08 2015
@@ -24,8 +24,8 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 
+import java.io.IOException;
 import java.io.InputStream;
-import java.util.Random;
 
 import com.google.common.base.Strings;
 import org.apache.jackrabbit.oak.plugins.segment.Segment;
@@ -40,7 +40,7 @@ import org.junit.rules.TemporaryFolder;
 public class ExternalBlobReferenceTest {
 
     @Rule
-    public TemporaryFolder segmentFolder = new TemporaryFolder();
+    public final TemporaryFolder segmentFolder = new TemporaryFolder();
 
     private FileStore fileStore;
 
@@ -53,15 +53,15 @@ public class ExternalBlobReferenceTest {
     }
 
     @After
-    public void destroyFileStore() throws Exception {
+    public void destroyFileStore() {
         fileStore.close();
     }
 
     /**
      * The {@code SegmentWriter} should be able to write blob IDs whose length
-     * is between 0 and 4095 bytes. It should be possible to correctly read the
-     * blob ID back and pass it to the {@code BlobStore} to obtain information
-     * about the blob.
+     * is between 0 and {@code Segment.BLOB_ID_SMALL_LIMIT - 1} bytes. It 
should
+     * be possible to correctly read the blob ID back and pass it to the {@code
+     * BlobStore} to obtain information about the blob.
      * <p/>
      * This code path executes only if the written stream is {@code
      * Segment.MEDIUM_LIMIT} bytes long (or more). If the length of the stream
@@ -71,59 +71,85 @@ public class ExternalBlobReferenceTest {
      * See OAK-3105.
      */
     @Test
-    public void testBlobIdLengthUpperLimit() throws Exception {
-        String blobId = Strings.repeat("x", 4095);
-        long blobLength = Segment.MEDIUM_LIMIT;
-
-        doReturn(blobId).when(blobStore).writeBlob(any(InputStream.class));
-        doReturn(blobLength).when(blobStore).getBlobLength(blobId);
-
-        SegmentBlob blob = 
fileStore.getTracker().getWriter().writeStream(newRandomInputStream(blobLength));
-
-        assertEquals(blobLength, blob.length());
+    public void testShortBlobId() throws Exception {
+        testBlobIdWithLength(Segment.BLOB_ID_SMALL_LIMIT - 1);
     }
 
     /**
-     * If the {@code BlobStore} returns a blob ID whose length is 4096 byes 
long
-     * (or more), writing the stream should throw an {@code
-     * IllegalArgumentException}.
+     * If the {@code BlobStore} returns a blob ID whose length is {@code
+     * Segment.BLOB_ID_SMALL_LIMIT} bytes long (or more), writing the stream
+     * should succeed. In this case, the blob ID is considered a long blob ID
+     * and an alternate encoding is used. It should be possible to correctly
+     * read the blob ID back and pass it to the {@code BlobStore} to obtain
+     * information about the blob.
      * <p/>
      * This code path executes only if the written stream is {@code
      * Segment.MEDIUM_LIMIT} bytes long (or more). If the length of the stream
      * is smaller, the binary value is inlined in the segment and the {@code
      * BlobStore} is never called.
      * <p/>
-     * See OAK-3105.
+     * See OAK-3105 and OAK-3107.
      */
-    @Test(expected = IllegalArgumentException.class)
-    public void testBlobIdLengthLongerThanUpperLimit() throws Exception {
-        String blobId = Strings.repeat("x", 4096);
+    @Test
+    public void testLongBlobId() throws Exception {
+        testBlobIdWithLength(Segment.BLOB_ID_SMALL_LIMIT);
+    }
+
+    private void testBlobIdWithLength(int blobIdLength) throws Exception {
+        String blobId = Strings.repeat("x", blobIdLength);
         long blobLength = Segment.MEDIUM_LIMIT;
 
         doReturn(blobId).when(blobStore).writeBlob(any(InputStream.class));
+        doReturn(blobLength).when(blobStore).getBlobLength(blobId);
+
+        SegmentBlob blob = 
fileStore.getTracker().getWriter().writeStream(newRandomInputStream(blobLength));
+
+        assertEquals(blobLength, blob.length());
+    }
+
+    private static InputStream newRandomInputStream(long size) {
+        return new LimitInputStream(new ConstantInputStream(0), size);
+    }
+
+    private static class ConstantInputStream extends InputStream {
+
+        private final int value;
+
+        public ConstantInputStream(int value) {
+            this.value = value;
+        }
+
+        @Override
+        public int read() {
+            return value;
+        }
 
-        
fileStore.getTracker().getWriter().writeStream(newRandomInputStream(blobLength));
     }
 
-    private InputStream newRandomInputStream(final long size) {
-        return new InputStream() {
+    private static class LimitInputStream extends InputStream {
 
-            private Random random = new Random();
+        private final InputStream stream;
 
-            private int read = 0;
+        private final long limit;
 
-            @Override
-            public int read() {
-                if (read >= size) {
-                    return -1;
-                }
+        private long read = 0;
 
-                read += 1;
+        public LimitInputStream(InputStream stream, long limit) {
+            this.stream = stream;
+            this.limit = limit;
+        }
 
-                return random.nextInt() & 0xFF;
+        @Override
+        public int read() throws IOException {
+            if (read >= limit) {
+                return -1;
             }
 
-        };
+            read = read + 1;
+
+            return stream.read();
+        }
+
     }
 
 }


Reply via email to