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 ad...@apache.org on 2023/11/07 10:29:52 UTC
(jackrabbit-oak) branch trunk updated: OAK-10511 - Get blobId without network roundtrip (#1171)
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 <ah...@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 <ah...@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;
+ }
}
}