Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Compressors from CompressorFactory to CompressorRegistry for extensibility #9262

Merged
merged 8 commits into from
Aug 15, 2023
Prev Previous commit
remove NONE singleton in CompressorRegistry and remove static block init
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
nknize committed Aug 14, 2023

Verified

This commit was signed with the committer’s verified signature.
nknize Nick Knize
commit f637245075f321920c91f3847995b2a368ee49f9
Original file line number Diff line number Diff line change
@@ -31,23 +31,19 @@
*/
@InternalApi
public final class CompressorRegistry {
/** No compression singleton - we still register so users can specify NONE in the API*/
public static final Compressor NONE;

// the backing registry map
private static final Map<String, Compressor> registeredCompressors;
private static final Map<String, Compressor> registeredCompressors = ServiceLoader.load(
CompressorProvider.class,
CompressorProvider.class.getClassLoader()
)
.stream()
.flatMap(p -> p.get().getCompressors().stream())
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));

// no instance:
private CompressorRegistry() {}

static {
registeredCompressors = ServiceLoader.load(CompressorProvider.class, CompressorProvider.class.getClassLoader())
.stream()
.flatMap(p -> p.get().getCompressors().stream())
.collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue));
NONE = registeredCompressors.get(NoneCompressor.NAME);
}

/**
* Returns the default compressor
*/
Original file line number Diff line number Diff line change
@@ -402,7 +402,7 @@ protected BlobStoreRepository(
cacheRepositoryData = CACHE_REPOSITORY_DATA.get(metadata.settings());
bufferSize = Math.toIntExact(BUFFER_SIZE_SETTING.get(metadata.settings()).getBytes());
maxShardBlobDeleteBatch = MAX_SNAPSHOT_SHARD_BLOB_DELETE_BATCH_SIZE.get(metadata.settings());
this.compressor = compress ? COMPRESSION_TYPE_SETTING.get(metadata.settings()) : CompressorRegistry.NONE;
this.compressor = compress ? COMPRESSION_TYPE_SETTING.get(metadata.settings()) : CompressorRegistry.none();
}

@Override
@@ -770,7 +770,7 @@ public BlobStore blobStore() {
* @return true if compression is needed
*/
protected final boolean isCompress() {
return compressor != CompressorRegistry.NONE;
return compressor != CompressorRegistry.none();
}

/**
Original file line number Diff line number Diff line change
@@ -119,7 +119,7 @@ public void testBlobStoreOperations() throws IOException {
ChecksumBlobStoreFormat<BlobObj> checksumSMILE = new ChecksumBlobStoreFormat<>(BLOB_CODEC, "%s", BlobObj::fromXContent);

// Write blobs in different formats
checksumSMILE.write(new BlobObj("checksum smile"), blobContainer, "check-smile", CompressorRegistry.NONE);
checksumSMILE.write(new BlobObj("checksum smile"), blobContainer, "check-smile", CompressorRegistry.none());
checksumSMILE.write(
new BlobObj("checksum smile compressed"),
blobContainer,
@@ -142,7 +142,7 @@ public void testCompressionIsApplied() throws IOException {
ChecksumBlobStoreFormat<BlobObj> checksumFormat = new ChecksumBlobStoreFormat<>(BLOB_CODEC, "%s", BlobObj::fromXContent);
BlobObj blobObj = new BlobObj(veryRedundantText.toString());
checksumFormat.write(blobObj, blobContainer, "blob-comp", CompressorRegistry.getCompressor(DeflateCompressor.NAME));
checksumFormat.write(blobObj, blobContainer, "blob-not-comp", CompressorRegistry.NONE);
checksumFormat.write(blobObj, blobContainer, "blob-not-comp", CompressorRegistry.none());
Map<String, BlobMetadata> blobs = blobContainer.listBlobsByPrefix("blob-");
assertEquals(blobs.size(), 2);
assertThat(blobs.get("blob-not-comp").length(), greaterThan(blobs.get("blob-comp").length()));