This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 23d311db5a5adfaee4a4a4b0490250907e820be0 Author: Benoit Tellier <[email protected]> AuthorDate: Tue Jan 7 14:05:18 2020 +0700 JAMES-2921 configure hybrid blobStore threshold --- .../destination/conf/blob.properties | 6 ++ .../destination/conf/blob.properties | 6 ++ server/blob/blob-union/pom.xml | 4 ++ .../apache/james/blob/union/HybridBlobStore.java | 67 +++++++++++++++++++--- .../james/blob/union/HybridBlobStoreTest.java | 22 ++++++- .../modules/blobstore/BlobStoreChoosingModule.java | 15 ++++- .../blobstore/BlobStoreChoosingModuleTest.java | 58 ++++++++++++++++++- 7 files changed, 164 insertions(+), 14 deletions(-) diff --git a/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/blob.properties b/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/blob.properties index f00fb5d..ea41c7f 100644 --- a/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/blob.properties +++ b/dockerfiles/run/guice/cassandra-rabbitmq-ldap/destination/conf/blob.properties @@ -6,6 +6,12 @@ # hybrid is using both objectstorage for unfrequently read or big blobs & cassandra for small, often read blobs implementation=objectstorage +# ========================================= Hybrid BlobStore ====================================== +# hybrid is using both objectstorage for unfrequently read or big blobs & cassandra for small, often read blobs +# Size threshold for considering a blob as 'big', causing it to be saved in the low cost blobStore +# Optional, defaults to 32768 bytes (32KB), must be positive +hybrid.size.threshold=32768 + # ============================================== ObjectStorage ============================================ # ========================================= ObjectStorage Codec ====================================== diff --git a/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/blob.properties b/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/blob.properties index f00fb5d..ea41c7f 100644 --- a/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/blob.properties +++ b/dockerfiles/run/guice/cassandra-rabbitmq/destination/conf/blob.properties @@ -6,6 +6,12 @@ # hybrid is using both objectstorage for unfrequently read or big blobs & cassandra for small, often read blobs implementation=objectstorage +# ========================================= Hybrid BlobStore ====================================== +# hybrid is using both objectstorage for unfrequently read or big blobs & cassandra for small, often read blobs +# Size threshold for considering a blob as 'big', causing it to be saved in the low cost blobStore +# Optional, defaults to 32768 bytes (32KB), must be positive +hybrid.size.threshold=32768 + # ============================================== ObjectStorage ============================================ # ========================================= ObjectStorage Codec ====================================== diff --git a/server/blob/blob-union/pom.xml b/server/blob/blob-union/pom.xml index 9df5cee..0e868fa 100644 --- a/server/blob/blob-union/pom.xml +++ b/server/blob/blob-union/pom.xml @@ -67,6 +67,10 @@ <scope>test</scope> </dependency> <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-configuration2</artifactId> + </dependency> + <dependency> <groupId>org.junit.jupiter</groupId> <artifactId>junit-jupiter-params</artifactId> <scope>test</scope> diff --git a/server/blob/blob-union/src/main/java/org/apache/james/blob/union/HybridBlobStore.java b/server/blob/blob-union/src/main/java/org/apache/james/blob/union/HybridBlobStore.java index 45d1813..03328a0 100644 --- a/server/blob/blob-union/src/main/java/org/apache/james/blob/union/HybridBlobStore.java +++ b/server/blob/blob-union/src/main/java/org/apache/james/blob/union/HybridBlobStore.java @@ -22,6 +22,8 @@ package org.apache.james.blob.union; import java.io.BufferedInputStream; import java.io.IOException; import java.io.InputStream; +import java.util.Objects; +import java.util.Optional; import org.apache.james.blob.api.BlobId; import org.apache.james.blob.api.BlobStore; @@ -43,43 +45,90 @@ public class HybridBlobStore implements BlobStore { @FunctionalInterface public interface RequirePerforming { - Builder highPerformance(BlobStore blobStore); + RequireConfiguration highPerformance(BlobStore blobStore); + } + + @FunctionalInterface + public interface RequireConfiguration { + Builder configuration(Configuration configuration); } public static class Builder { private final BlobStore lowCostBlobStore; private final BlobStore highPerformanceBlobStore; + private final Configuration configuration; - Builder(BlobStore lowCostBlobStore, BlobStore highPerformanceBlobStore) { + Builder(BlobStore lowCostBlobStore, BlobStore highPerformanceBlobStore, Configuration configuration) { this.lowCostBlobStore = lowCostBlobStore; this.highPerformanceBlobStore = highPerformanceBlobStore; + this.configuration = configuration; } public HybridBlobStore build() { return new HybridBlobStore( lowCostBlobStore, - highPerformanceBlobStore); + highPerformanceBlobStore, + configuration); + } + } + + public static class Configuration { + public static final int DEFAULT_SIZE_THREASHOLD = 32 * 1024; + public static final Configuration DEFAULT = new Configuration(DEFAULT_SIZE_THREASHOLD); + private static final String PROPERTY_NAME = "hybrid.size.threshold"; + + public static Configuration from(org.apache.commons.configuration2.Configuration propertiesConfiguration) { + return new Configuration(Optional.ofNullable(propertiesConfiguration.getInteger(PROPERTY_NAME, null)) + .orElse(DEFAULT_SIZE_THREASHOLD)); + } + + private final int sizeThreshold; + + public Configuration(int sizeThreshold) { + Preconditions.checkArgument(sizeThreshold >= 0, "'" + PROPERTY_NAME + "' needs to be positive"); + + this.sizeThreshold = sizeThreshold; + } + + public int getSizeThreshold() { + return sizeThreshold; + } + + @Override + public final boolean equals(Object o) { + if (o instanceof Configuration) { + Configuration that = (Configuration) o; + + return Objects.equals(this.sizeThreshold, that.sizeThreshold); + } + return false; + } + + @Override + public final int hashCode() { + return Objects.hash(sizeThreshold); } } private static final Logger LOGGER = LoggerFactory.getLogger(HybridBlobStore.class); - private static final int SIZE_THRESHOLD = 32 * 1024; public static RequireLowCost builder() { - return lowCost -> highPerformance -> new Builder(lowCost, highPerformance); + return lowCost -> highPerformance -> configuration -> new Builder(lowCost, highPerformance, configuration); } private final BlobStore lowCostBlobStore; private final BlobStore highPerformanceBlobStore; + private final Configuration configuration; - private HybridBlobStore(BlobStore lowCostBlobStore, BlobStore highPerformanceBlobStore) { + private HybridBlobStore(BlobStore lowCostBlobStore, BlobStore highPerformanceBlobStore, Configuration configuration) { this.lowCostBlobStore = lowCostBlobStore; this.highPerformanceBlobStore = highPerformanceBlobStore; + this.configuration = configuration; } @Override public Mono<BlobId> save(BucketName bucketName, byte[] data, StoragePolicy storagePolicy) { - return selectBlobStore(storagePolicy, Mono.just(data.length > SIZE_THRESHOLD)) + return selectBlobStore(storagePolicy, Mono.just(data.length > configuration.getSizeThreshold())) .flatMap(blobStore -> blobStore.save(bucketName, data, storagePolicy)); } @@ -87,7 +136,7 @@ public class HybridBlobStore implements BlobStore { public Mono<BlobId> save(BucketName bucketName, InputStream data, StoragePolicy storagePolicy) { Preconditions.checkNotNull(data); - BufferedInputStream bufferedInputStream = new BufferedInputStream(data, SIZE_THRESHOLD + 1); + BufferedInputStream bufferedInputStream = new BufferedInputStream(data, configuration.getSizeThreshold() + 1); return selectBlobStore(storagePolicy, Mono.fromCallable(() -> isItABigStream(bufferedInputStream))) .flatMap(blobStore -> blobStore.save(bucketName, bufferedInputStream, storagePolicy)); } @@ -112,7 +161,7 @@ public class HybridBlobStore implements BlobStore { private boolean isItABigStream(InputStream bufferedData) throws IOException { bufferedData.mark(0); - bufferedData.skip(SIZE_THRESHOLD); + bufferedData.skip(configuration.getSizeThreshold()); boolean isItABigStream = bufferedData.read() != -1; bufferedData.reset(); return isItABigStream; diff --git a/server/blob/blob-union/src/test/java/org/apache/james/blob/union/HybridBlobStoreTest.java b/server/blob/blob-union/src/test/java/org/apache/james/blob/union/HybridBlobStoreTest.java index 97bb738..ad9d38f 100644 --- a/server/blob/blob-union/src/test/java/org/apache/james/blob/union/HybridBlobStoreTest.java +++ b/server/blob/blob-union/src/test/java/org/apache/james/blob/union/HybridBlobStoreTest.java @@ -19,8 +19,8 @@ package org.apache.james.blob.union; -import static org.apache.james.blob.api.BlobStore.StoragePolicy.LOW_COST; import static org.apache.james.blob.api.BlobStore.StoragePolicy.HIGH_PERFORMANCE; +import static org.apache.james.blob.api.BlobStore.StoragePolicy.LOW_COST; import static org.apache.james.blob.api.BlobStore.StoragePolicy.SIZE_BASED; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -45,6 +45,7 @@ import org.junit.jupiter.api.Test; import com.github.fge.lambdas.Throwing; import com.google.common.base.MoreObjects; +import nl.jqno.equalsverifier.EqualsVerifier; import reactor.core.publisher.Mono; class HybridBlobStoreTest implements BlobStoreContract { @@ -161,6 +162,7 @@ class HybridBlobStoreTest implements BlobStoreContract { hybridBlobStore = HybridBlobStore.builder() .lowCost(lowCostBlobStore) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); } @@ -281,6 +283,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new ThrowingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); assertThatThrownBy(() -> hybridBlobStore.save(hybridBlobStore.getDefaultBucketName(), BLOB_CONTENT, LOW_COST).block()) @@ -293,6 +296,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new ThrowingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); assertThatThrownBy(() -> hybridBlobStore.save(hybridBlobStore.getDefaultBucketName(), new ByteArrayInputStream(BLOB_CONTENT), LOW_COST).block()) @@ -309,6 +313,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new FailingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); assertThatThrownBy(() -> hybridBlobStore.save(hybridBlobStore.getDefaultBucketName(), BLOB_CONTENT, LOW_COST).block()) @@ -321,6 +326,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new FailingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); assertThatThrownBy(() -> hybridBlobStore.save(hybridBlobStore.getDefaultBucketName(), new ByteArrayInputStream(BLOB_CONTENT), LOW_COST).block()) @@ -338,6 +344,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new ThrowingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); BlobId blobId = highPerformanceBlobStore.save(hybridBlobStore.getDefaultBucketName(), BLOB_CONTENT, LOW_COST).block(); @@ -352,6 +359,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new ThrowingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); BlobId blobId = highPerformanceBlobStore.save(hybridBlobStore.getDefaultBucketName(), BLOB_CONTENT, LOW_COST).block(); @@ -370,6 +378,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new FailingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); BlobId blobId = highPerformanceBlobStore.save(hybridBlobStore.getDefaultBucketName(), BLOB_CONTENT, LOW_COST).block(); @@ -383,6 +392,7 @@ class HybridBlobStoreTest implements BlobStoreContract { HybridBlobStore hybridBlobStore = HybridBlobStore.builder() .lowCost(new FailingBlobStore()) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); BlobId blobId = highPerformanceBlobStore.save(hybridBlobStore.getDefaultBucketName(), BLOB_CONTENT, LOW_COST).block(); @@ -469,6 +479,7 @@ class HybridBlobStoreTest implements BlobStoreContract { hybridBlobStore = HybridBlobStore.builder() .lowCost(lowCostBlobStore) .highPerformance(highPerformanceBlobStore) + .configuration(HybridBlobStore.Configuration.DEFAULT) .build(); assertThatThrownBy(() -> hybridBlobStore.getDefaultBucketName()) @@ -513,4 +524,13 @@ class HybridBlobStoreTest implements BlobStoreContract { assertThatCode(() -> hybridBlobStore.delete(BucketName.DEFAULT, blobIdFactory().randomId()).block()) .doesNotThrowAnyException(); } + + @Nested + class ConfigurationTest { + @Test + void shouldMatchBeanContract() { + EqualsVerifier.forClass(HybridBlobStore.Configuration.class) + .verify(); + } + } } diff --git a/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/blobstore/BlobStoreChoosingModule.java b/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/blobstore/BlobStoreChoosingModule.java index 2ab354d..82e74cf 100644 --- a/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/blobstore/BlobStoreChoosingModule.java +++ b/server/container/guice/cassandra-rabbitmq-guice/src/main/java/org/apache/james/modules/blobstore/BlobStoreChoosingModule.java @@ -75,7 +75,8 @@ public class BlobStoreChoosingModule extends AbstractModule { @Singleton BlobStore provideBlobStore(BlobStoreChoosingConfiguration choosingConfiguration, Provider<CassandraBlobStore> cassandraBlobStoreProvider, - Provider<ObjectStorageBlobStore> swiftBlobStoreProvider) { + Provider<ObjectStorageBlobStore> swiftBlobStoreProvider, + HybridBlobStore.Configuration hybridBlobStoreConfiguration) { switch (choosingConfiguration.getImplementation()) { case OBJECTSTORAGE: @@ -86,6 +87,7 @@ public class BlobStoreChoosingModule extends AbstractModule { return HybridBlobStore.builder() .lowCost(swiftBlobStoreProvider.get()) .highPerformance(cassandraBlobStoreProvider.get()) + .configuration(hybridBlobStoreConfiguration) .build(); default: throw new RuntimeException(String.format("can not get the right blobstore provider with configuration %s", @@ -93,4 +95,15 @@ public class BlobStoreChoosingModule extends AbstractModule { } } + @Provides + @Singleton + @VisibleForTesting + HybridBlobStore.Configuration providesHybridBlobStoreConfiguration(PropertiesProvider propertiesProvider) { + try { + Configuration configuration = propertiesProvider.getConfigurations(ConfigurationComponent.NAMES); + return HybridBlobStore.Configuration.from(configuration); + } catch (FileNotFoundException | ConfigurationException e) { + return HybridBlobStore.Configuration.DEFAULT; + } + } } diff --git a/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/blobstore/BlobStoreChoosingModuleTest.java b/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/blobstore/BlobStoreChoosingModuleTest.java index 4e04a5b..810cf50 100644 --- a/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/blobstore/BlobStoreChoosingModuleTest.java +++ b/server/container/guice/cassandra-rabbitmq-guice/src/test/java/org/apache/james/modules/blobstore/BlobStoreChoosingModuleTest.java @@ -131,11 +131,63 @@ class BlobStoreChoosingModuleTest { } @Test + void providesHybridBlobStoreConfigurationShouldThrowWhenNegative() { + BlobStoreChoosingModule module = new BlobStoreChoosingModule(); + PropertiesConfiguration configuration = new PropertiesConfiguration(); + configuration.addProperty("hybrid.size.threshold", -1); + FakePropertiesProvider propertyProvider = FakePropertiesProvider.builder() + .register(ConfigurationComponent.NAME, configuration) + .build(); + + assertThatThrownBy(() -> module.providesHybridBlobStoreConfiguration(propertyProvider)) + .isInstanceOf(IllegalArgumentException.class); + } + + @Test + void providesHybridBlobStoreConfigurationShouldNotThrowWhenZero() { + BlobStoreChoosingModule module = new BlobStoreChoosingModule(); + PropertiesConfiguration configuration = new PropertiesConfiguration(); + configuration.addProperty("hybrid.size.threshold", 0); + FakePropertiesProvider propertyProvider = FakePropertiesProvider.builder() + .register(ConfigurationComponent.NAME, configuration) + .build(); + + assertThat(module.providesHybridBlobStoreConfiguration(propertyProvider)) + .isEqualTo(new HybridBlobStore.Configuration(0)); + } + + @Test + void providesHybridBlobStoreConfigurationShouldReturnConfiguration() { + BlobStoreChoosingModule module = new BlobStoreChoosingModule(); + PropertiesConfiguration configuration = new PropertiesConfiguration(); + configuration.addProperty("hybrid.size.threshold", 36); + FakePropertiesProvider propertyProvider = FakePropertiesProvider.builder() + .register(ConfigurationComponent.NAME, configuration) + .build(); + + assertThat(module.providesHybridBlobStoreConfiguration(propertyProvider)) + .isEqualTo(new HybridBlobStore.Configuration(36)); + } + + @Test + void providesHybridBlobStoreConfigurationShouldReturnConfigurationWhenLegacyFile() { + BlobStoreChoosingModule module = new BlobStoreChoosingModule(); + PropertiesConfiguration configuration = new PropertiesConfiguration(); + configuration.addProperty("hybrid.size.threshold", 36); + FakePropertiesProvider propertyProvider = FakePropertiesProvider.builder() + .register(ConfigurationComponent.LEGACY, configuration) + .build(); + + assertThat(module.providesHybridBlobStoreConfiguration(propertyProvider)) + .isEqualTo(new HybridBlobStore.Configuration(36)); + } + + @Test void provideBlobStoreShouldReturnCassandraBlobStoreWhenCassandraConfigured() { BlobStoreChoosingModule module = new BlobStoreChoosingModule(); assertThat(module.provideBlobStore(BlobStoreChoosingConfiguration.cassandra(), - CASSANDRA_BLOBSTORE_PROVIDER, OBJECT_STORAGE_BLOBSTORE_PROVIDER)) + CASSANDRA_BLOBSTORE_PROVIDER, OBJECT_STORAGE_BLOBSTORE_PROVIDER, HybridBlobStore.Configuration.DEFAULT)) .isEqualTo(CASSANDRA_BLOBSTORE); } @@ -144,7 +196,7 @@ class BlobStoreChoosingModuleTest { BlobStoreChoosingModule module = new BlobStoreChoosingModule(); assertThat(module.provideBlobStore(BlobStoreChoosingConfiguration.cassandra(), - CASSANDRA_BLOBSTORE_PROVIDER, OBJECT_STORAGE_BLOBSTORE_PROVIDER)) + CASSANDRA_BLOBSTORE_PROVIDER, OBJECT_STORAGE_BLOBSTORE_PROVIDER, HybridBlobStore.Configuration.DEFAULT)) .isEqualTo(CASSANDRA_BLOBSTORE); } @@ -153,7 +205,7 @@ class BlobStoreChoosingModuleTest { BlobStoreChoosingModule module = new BlobStoreChoosingModule(); assertThat(module.provideBlobStore(BlobStoreChoosingConfiguration.hybrid(), - CASSANDRA_BLOBSTORE_PROVIDER, OBJECT_STORAGE_BLOBSTORE_PROVIDER)) + CASSANDRA_BLOBSTORE_PROVIDER, OBJECT_STORAGE_BLOBSTORE_PROVIDER, HybridBlobStore.Configuration.DEFAULT)) .isInstanceOf(HybridBlobStore.class); } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
