This is an automated email from the ASF dual-hosted git repository. adulceanu pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push: new 0de5e850ec OAK-10511 - Get blobId without network roundtrip (#1171) 0de5e850ec is described below commit 0de5e850ec8f7a0ca482529e4adbe20f8ef8afa2 Author: Axel Hanikel <ahani...@gmail.com> AuthorDate: Tue Nov 7 11:29:46 2023 +0100 OAK-10511 - Get blobId without network roundtrip (#1171) * OAK-10511 - Get blobId without network roundtrip * Add a test * Lazy init: as in https://stackoverflow.com/a/3580658 --------- Co-authored-by: Axel Hanikel <ahani...@adobe.com> --- .../oak/segment/DefaultSegmentWriter.java | 8 +++++ .../oak/segment/DefaultSegmentWriterTest.java | 40 +++++++++++++++++++++- .../oak/segment/test/TemporaryBlobStore.java | 33 +++++++++++++++--- .../oak/segment/test/TemporaryFileStore.java | 37 ++++++++++++++++---- 4 files changed, 105 insertions(+), 13 deletions(-) diff --git a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java index 3211151ef8..edef45810b 100644 --- a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java +++ b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java @@ -65,6 +65,7 @@ import org.apache.jackrabbit.oak.api.Blob; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.Buffer; +import org.apache.jackrabbit.oak.plugins.blob.BlobStoreBlob; import org.apache.jackrabbit.oak.plugins.memory.ModifiedNodeState; import org.apache.jackrabbit.oak.segment.RecordWriters.RecordWriter; import org.apache.jackrabbit.oak.segment.WriteOperationHandler.WriteOperation; @@ -576,6 +577,13 @@ public class DefaultSegmentWriter implements SegmentWriter { } } + if (blob instanceof BlobStoreBlob) { + String blobId = ((BlobStoreBlob) blob).getBlobId(); + if (blobId != null) { + return writeBlobId(blobId); + } + } + String reference = blob.getReference(); if (reference != null && blobStore != null) { String blobId = blobStore.getBlobId(reference); diff --git a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java index 49e7ebb795..d2e96c1707 100644 --- a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java +++ b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java @@ -24,10 +24,12 @@ import static org.apache.jackrabbit.guava.common.collect.Maps.newHashMap; import static org.apache.jackrabbit.oak.segment.DefaultSegmentWriterBuilder.defaultSegmentWriterBuilder; import static org.apache.jackrabbit.oak.segment.ListRecord.LEVEL_SIZE; import static org.apache.jackrabbit.oak.segment.ListRecord.MAX_ELEMENTS; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.File; @@ -40,6 +42,10 @@ import java.util.List; import java.util.Map; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.RandomUtils; +import org.apache.jackrabbit.oak.api.Blob; +import org.apache.jackrabbit.oak.plugins.blob.BlobStoreBlob; +import org.apache.jackrabbit.oak.segment.test.TemporaryBlobStore; import org.apache.jackrabbit.oak.segment.test.TemporaryFileStore; import org.jetbrains.annotations.NotNull; import org.junit.Before; @@ -68,13 +74,24 @@ public class DefaultSegmentWriterTest { private static final int SMALL_BINARIES_INLINE_THRESHOLD = 4; + private TemporaryFolder blobFolder = new TemporaryFolder(new File("target")); + + private TemporaryBlobStore blobStore = new TemporaryBlobStore(blobFolder); + private TemporaryFolder folder = new TemporaryFolder(new File("target")); - private TemporaryFileStore store = new TemporaryFileStore(folder, SMALL_BINARIES_INLINE_THRESHOLD); + private TemporaryFileStore store = new TemporaryFileStore(folder, null, SMALL_BINARIES_INLINE_THRESHOLD); + + private TemporaryFileStore storeWithBlobStore = new TemporaryFileStore(folder, blobStore, SMALL_BINARIES_INLINE_THRESHOLD); @Rule public RuleChain rules = RuleChain.outerRule(folder).around(store); + @Rule + public RuleChain rulesWithBlobStore = RuleChain + .outerRule(blobFolder).around(blobStore) + .around(folder).around(storeWithBlobStore); + private DefaultSegmentWriter writer; @Before @@ -390,6 +407,27 @@ public class DefaultSegmentWriterTest { } } + @Test + public void testWriteBlobWithBlobStoreBlob() throws IOException { + writer = defaultSegmentWriterBuilder("test").build(storeWithBlobStore.fileStore()); + byte[] randomBytes = RandomUtils.nextBytes(16384); + InputStream is = new ByteArrayInputStream(randomBytes); + String blobId = blobStore.blobStore().writeBlob(is); + Blob theBlob = new BlobStoreBlob(blobStore.blobStore(), blobId) { + @Override + public String getReference() { + throw new IllegalStateException("blobId should have been fetched from the blob instance."); + } + }; + try { + RecordId valueId = writer.writeBlob(theBlob); + SegmentBlob blob = new SegmentBlob(blobStore.blobStore(), valueId); + assertArrayEquals(randomBytes, IOUtils.toByteArray(blob.getNewStream())); + } catch (IllegalStateException e) { + fail(e.getMessage()); + } + } + private ListAppender<ILoggingEvent> subscribeAppender() { ListAppender<ILoggingEvent> appender = new ListAppender<ILoggingEvent>(); appender.setContext((LoggerContext) LoggerFactory.getILoggerFactory()); diff --git a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryBlobStore.java b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryBlobStore.java index adce632623..7c47de8c50 100644 --- a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryBlobStore.java +++ b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryBlobStore.java @@ -24,13 +24,15 @@ import org.apache.jackrabbit.oak.spi.blob.BlobStore; import org.junit.rules.ExternalResource; import org.junit.rules.TemporaryFolder; +import java.io.IOException; + public class TemporaryBlobStore extends ExternalResource { private final TemporaryFolder folder; private final String name; - private DataStoreBlobStore store; + private volatile DataStoreBlobStore store; public TemporaryBlobStore(TemporaryFolder folder) { this(folder, null); @@ -41,14 +43,18 @@ public class TemporaryBlobStore extends ExternalResource { this.name = name; } - @Override - protected void before() throws Throwable { + protected void init() throws IOException { FileDataStore fds = new FileDataStore(); configureDataStore(fds); fds.init((name == null ? folder.newFolder() : folder.newFolder(name)).getAbsolutePath()); store = new DataStoreBlobStore(fds); } + @Override + protected void before() throws Throwable { + // delay initialisation until the store is accessed + } + protected void configureDataStore(FileDataStore dataStore) { dataStore.setMinRecordLength(4092); } @@ -56,14 +62,31 @@ public class TemporaryBlobStore extends ExternalResource { @Override protected void after() { try { - store.close(); + if (store != null) { + store.close(); + } } catch (DataStoreException e) { throw new IllegalStateException(e); } } public BlobStore blobStore() { - return store; + // We use the local variable so we access the volatile {this.store} only once. + // see: https://stackoverflow.com/a/3580658 + DataStoreBlobStore store = this.store; + if (store != null) { + return store; + } + synchronized (this) { + if (this.store == null) { + try { + init(); + } catch (IOException e) { + throw new IllegalStateException("Initialisation failed", e); + } + } + return this.store; + } } } diff --git a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryFileStore.java b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryFileStore.java index 9a7f1b6f36..6b8b0274b4 100644 --- a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryFileStore.java +++ b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/test/TemporaryFileStore.java @@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.segment.test; import static org.apache.jackrabbit.oak.segment.file.FileStoreBuilder.fileStoreBuilder; +import java.io.IOException; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; @@ -28,6 +29,7 @@ import org.apache.jackrabbit.oak.segment.SegmentNotFoundExceptionListener; import org.apache.jackrabbit.oak.segment.compaction.SegmentGCOptions; import org.apache.jackrabbit.oak.segment.file.FileStore; import org.apache.jackrabbit.oak.segment.file.FileStoreBuilder; +import org.apache.jackrabbit.oak.segment.file.InvalidFileStoreVersionException; import org.apache.jackrabbit.oak.stats.DefaultStatisticsProvider; import org.junit.rules.ExternalResource; import org.junit.rules.TemporaryFolder; @@ -44,7 +46,7 @@ public class TemporaryFileStore extends ExternalResource { private ScheduledExecutorService executor; - private FileStore store; + private volatile FileStore store; private int binariesInlineThreshold = Segment.MEDIUM_LIMIT; @@ -63,13 +65,12 @@ public class TemporaryFileStore extends ExternalResource { this.name = name; } - public TemporaryFileStore(TemporaryFolder folder, int binariesInlineThreshold) { - this(folder, null, false, null); + public TemporaryFileStore(TemporaryFolder folder, TemporaryBlobStore blobStore, int binariesInlineThreshold) { + this(folder, blobStore, false, null); this.binariesInlineThreshold = binariesInlineThreshold; } - @Override - protected void before() throws Throwable { + protected void init() throws InvalidFileStoreVersionException, IOException { executor = Executors.newSingleThreadScheduledExecutor(); FileStoreBuilder builder = fileStoreBuilder(name == null ? folder.newFolder() : folder.newFolder(name)) .withMaxFileSize(1) @@ -94,17 +95,39 @@ public class TemporaryFileStore extends ExternalResource { store = builder.build(); } + @Override + protected void before() throws Throwable { + // delay initialisation until the store is accessed + } + @Override protected void after() { try { - store.close(); + if (store != null) { + store.close(); + } } finally { new ExecutorCloser(executor).close(); } } public FileStore fileStore() { - return store; + // We use the local variable so we access the volatile {this.store} only once. + // see: https://stackoverflow.com/a/3580658 + FileStore store = this.store; + if (store != null) { + return store; + } + synchronized (this) { + if (this.store == null) { + try { + init(); + } catch (InvalidFileStoreVersionException | IOException e) { + throw new IllegalStateException("Initialisation failed", e); + } + } + return this.store; + } } }