Add ZSTD compression for snapshotting (opensearch-project#2996)

Changes: - Added ZSTD compressor for snapshotting - 2 JSON repository settings: - readonly - compression were moved into the BlobStoreRepository class and removed from other repos classes where they were used. Signed-off-by: Andrey Pleskach <ples@aiven.io>
willyborankin · Jun 4, 2023 · 2c66e9e · 2c66e9e
1 parent 32ca5f9
commit 2c66e9e
Show file tree

Hide file tree

Showing 30 changed files with 737 additions and 462 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,11 +13,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Bump `org.gradle.test-retry` from 1.5.2 to 1.5.3 (#7810)
 - Bump `com.diffplug.spotless` from 6.17.0 to 6.18.0 (#7896)
 - Bump `jackson` from 2.15.1 to 2.15.2 ([#7897](https://github.com/opensearch-project/OpenSearch/pull/7897))
+- Add `com.github.luben:zstd-jni` version 1.5.5-3 ([#2996](https://github.com/opensearch-project/OpenSearch/pull/2996))
 
 ### Changed
 - Replace jboss-annotations-api_1.2_spec with jakarta.annotation-api ([#7836](https://github.com/opensearch-project/OpenSearch/pull/7836))
 - Reduce memory copy in zstd compression ([#7681](https://github.com/opensearch-project/OpenSearch/pull/7681))
 - Add min, max, average and thread info to resource stats in tasks API ([#7673](https://github.com/opensearch-project/OpenSearch/pull/7673))
+- Add ZSTD compression for snapshotting ([#2996](https://github.com/opensearch-project/OpenSearch/pull/2996))
 
 ### Deprecated
 

diff --git a/buildSrc/version.properties b/buildSrc/version.properties
@@ -54,3 +54,6 @@ bytebuddy         = 1.14.3
 
 # benchmark dependencies
 jmh               = 1.35
+
+# compression
+zstd              = 1.5.5-3
diff --git a/distribution/tools/plugin-cli/build.gradle b/distribution/tools/plugin-cli/build.gradle
@@ -82,9 +82,6 @@ thirdPartyAudit.ignoreViolations(
 )
 
 thirdPartyAudit.ignoreMissingClasses(
-  'com.github.luben.zstd.BufferPool',
-  'com.github.luben.zstd.ZstdInputStream',
-  'com.github.luben.zstd.ZstdOutputStream',
   'org.brotli.dec.BrotliInputStream',
   'org.objectweb.asm.AnnotationVisitor',
   'org.objectweb.asm.Attribute',

diff --git a/modules/transport-netty4/build.gradle b/modules/transport-netty4/build.gradle
@@ -184,7 +184,6 @@ thirdPartyAudit {
     'org.slf4j.LoggerFactory',
     'org.slf4j.spi.LocationAwareLogger',
 
-    'com.github.luben.zstd.Zstd',
     'com.google.protobuf.nano.CodedOutputByteBufferNano',
     'com.google.protobuf.nano.MessageNano',
     'com.jcraft.jzlib.Deflater',

diff --git a/...ins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureRepository.java b/...ins/repository-azure/src/main/java/org/opensearch/repositories/azure/AzureRepository.java
@@ -100,8 +100,6 @@ public static final class Repository {
             MAX_CHUNK_SIZE,
             Property.NodeScope
         );
-        public static final Setting<Boolean> COMPRESS_SETTING = Setting.boolSetting("compress", false, Property.NodeScope);
-        public static final Setting<Boolean> READONLY_SETTING = Setting.boolSetting("readonly", false, Property.NodeScope);
     }
 
     private final BlobPath basePath;
@@ -118,7 +116,7 @@ public AzureRepository(
     ) {
         super(
             metadata,
-            Repository.COMPRESS_SETTING.get(metadata.settings()),
+            COMPRESS_SETTING.get(metadata.settings()),
             namedXContentRegistry,
             clusterService,
             recoverySettings,
@@ -142,8 +140,8 @@ public AzureRepository(
         // If the user explicitly did not define a readonly value, we set it by ourselves depending on the location mode setting.
         // For secondary_only setting, the repository should be read only
         final LocationMode locationMode = Repository.LOCATION_MODE_SETTING.get(metadata.settings());
-        if (Repository.READONLY_SETTING.exists(metadata.settings())) {
-            this.readonly = Repository.READONLY_SETTING.get(metadata.settings());
+        if (READONLY_SETTING.exists(metadata.settings())) {
+            this.readonly = READONLY_SETTING.get(metadata.settings());
         } else {
             this.readonly = locationMode == LocationMode.SECONDARY_ONLY;
         }

diff --git a/...itory-gcs/src/main/java/org/opensearch/repositories/gcs/GoogleCloudStorageRepository.java b/...itory-gcs/src/main/java/org/opensearch/repositories/gcs/GoogleCloudStorageRepository.java
@@ -50,7 +50,6 @@
 import java.util.function.Function;
 
 import static org.opensearch.common.settings.Setting.Property;
-import static org.opensearch.common.settings.Setting.boolSetting;
 import static org.opensearch.common.settings.Setting.byteSizeSetting;
 import static org.opensearch.common.settings.Setting.simpleString;
 
@@ -70,7 +69,6 @@ class GoogleCloudStorageRepository extends MeteredBlobStoreRepository {
 
     static final Setting<String> BUCKET = simpleString("bucket", Property.NodeScope, Property.Dynamic);
     static final Setting<String> BASE_PATH = simpleString("base_path", Property.NodeScope, Property.Dynamic);
-    static final Setting<Boolean> COMPRESS = boolSetting("compress", false, Property.NodeScope, Property.Dynamic);
     static final Setting<ByteSizeValue> CHUNK_SIZE = byteSizeSetting(
         "chunk_size",
         MAX_CHUNK_SIZE,
@@ -94,7 +92,14 @@ class GoogleCloudStorageRepository extends MeteredBlobStoreRepository {
         final ClusterService clusterService,
         final RecoverySettings recoverySettings
     ) {
-        super(metadata, getSetting(COMPRESS, metadata), namedXContentRegistry, clusterService, recoverySettings, buildLocation(metadata));
+        super(
+            metadata,
+            getSetting(COMPRESS_SETTING, metadata),
+            namedXContentRegistry,
+            clusterService,
+            recoverySettings,
+            buildLocation(metadata)
+        );
         this.storageService = storageService;
 
         String basePath = BASE_PATH.get(metadata.settings());

diff --git a/plugins/repository-hdfs/src/main/java/org/opensearch/repositories/hdfs/HdfsRepository.java b/plugins/repository-hdfs/src/main/java/org/opensearch/repositories/hdfs/HdfsRepository.java
@@ -83,7 +83,7 @@ public HdfsRepository(
         final ClusterService clusterService,
         final RecoverySettings recoverySettings
     ) {
-        super(metadata, metadata.settings().getAsBoolean("compress", false), namedXContentRegistry, clusterService, recoverySettings);
+        super(metadata, COMPRESS_SETTING.get(metadata.settings()), namedXContentRegistry, clusterService, recoverySettings);
 
         this.environment = environment;
         this.chunkSize = metadata.settings().getAsBytesSize("chunk_size", null);

diff --git a/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java b/plugins/repository-s3/src/main/java/org/opensearch/repositories/s3/S3Repository.java
@@ -162,12 +162,6 @@ class S3Repository extends MeteredBlobStoreRepository {
         new ByteSizeValue(5, ByteSizeUnit.TB)
     );
 
-    /**
-     * When set to true metadata files are stored in compressed format. This setting doesn’t affect index
-     * files that are already compressed by default. Defaults to false.
-     */
-    static final Setting<Boolean> COMPRESS_SETTING = Setting.boolSetting("compress", false);
-
     /**
      * Sets the S3 storage class type for the backup files. Values may be standard, reduced_redundancy,
      * standard_ia, onezone_ia and intelligent_tiering. Defaults to standard.

diff --git a/plugins/transport-nio/build.gradle b/plugins/transport-nio/build.gradle
@@ -111,7 +111,6 @@ thirdPartyAudit {
     'org.slf4j.LoggerFactory',
     'org.slf4j.spi.LocationAwareLogger',
 
-    'com.github.luben.zstd.Zstd',
     'com.google.protobuf.nano.CodedOutputByteBufferNano',
     'com.google.protobuf.nano.MessageNano',
     'com.jcraft.jzlib.Deflater',

diff --git a/sandbox/plugins/custom-codecs/build.gradle b/sandbox/plugins/custom-codecs/build.gradle
@@ -21,7 +21,7 @@ opensearchplugin {
 }
 
 dependencies {
-  api "com.github.luben:zstd-jni:1.5.5-1"
+  api "com.github.luben:zstd-jni:${versions.zstd}"
 }
 
 yamlRestTest.enabled = false;

diff --git a/sandbox/plugins/custom-codecs/licenses/zstd-jni-1.5.5-1.jar.sha1 b/sandbox/plugins/custom-codecs/licenses/zstd-jni-1.5.5-1.jar.sha1
diff --git a/server/build.gradle b/server/build.gradle
@@ -150,6 +150,9 @@ dependencies {
   api "com.google.protobuf:protobuf-java:${versions.protobuf}"
   api "jakarta.annotation:jakarta.annotation-api:${versions.jakarta_annotation}"
 
+  //zstd
+  api "com.github.luben:zstd-jni:${versions.zstd}"
+
   testImplementation(project(":test:framework")) {
     // tests use the locally compiled version of server
     exclude group: 'org.opensearch', module: 'server'

diff --git a/server/licenses/zstd-jni-1.5.5-3.jar.sha1 b/server/licenses/zstd-jni-1.5.5-3.jar.sha1
@@ -0,0 +1 @@
+488dd9b15c9e8cf87d857f65f5cd6359c2853381
diff --git a/...stom-codecs/licenses/zstd-jni-LICENSE.txt → server/licenses/zstd-jni-LICENSE.txt b/...stom-codecs/licenses/zstd-jni-LICENSE.txt → server/licenses/zstd-jni-LICENSE.txt
diff --git a/...ustom-codecs/licenses/zstd-jni-NOTICE.txt → server/licenses/zstd-jni-NOTICE.txt b/...ustom-codecs/licenses/zstd-jni-NOTICE.txt → server/licenses/zstd-jni-NOTICE.txt
diff --git a/server/src/main/java/org/opensearch/common/compress/CompressorFactory.java b/server/src/main/java/org/opensearch/common/compress/CompressorFactory.java
@@ -47,7 +47,14 @@
  */
 public class CompressorFactory {
 
-    public static final Compressor COMPRESSOR = new DeflateCompressor();
+    public static final Compressor DEFLATE_COMPRESSOR = new DeflateCompressor();
+
+    @Deprecated
+    public static final Compressor COMPRESSOR = DEFLATE_COMPRESSOR;
+
+    public static final Compressor ZSTD_COMPRESSOR = new ZstdCompressor();
+
+    public static final Compressor NONE_COMPRESSOR = new NoneCompressor();
 
     public static boolean isCompressed(BytesReference bytes) {
         return compressor(bytes) != null;
@@ -61,6 +68,9 @@ public static Compressor compressor(BytesReference bytes) {
             // as a xcontent, we have a problem
             assert XContentHelper.xContentType(bytes) == null;
             return COMPRESSOR;
+        } else if (ZSTD_COMPRESSOR.isCompressed(bytes)) {
+            assert XContentHelper.xContentType(bytes) == null;
+            return ZSTD_COMPRESSOR;
         }
 
         XContentType contentType = XContentHelper.xContentType(bytes);
@@ -81,7 +91,6 @@ private static boolean isAncient(BytesReference bytes) {
 
     /**
      * Uncompress the provided data, data can be detected as compressed using {@link #isCompressed(BytesReference)}.
-     * @throws NullPointerException a NullPointerException will be thrown when bytes is null
      */
     public static BytesReference uncompressIfNeeded(BytesReference bytes) throws IOException {
         Compressor compressor = compressor(Objects.requireNonNull(bytes, "the BytesReference must not be null"));

diff --git a/server/src/main/java/org/opensearch/common/compress/CompressorType.java b/server/src/main/java/org/opensearch/common/compress/CompressorType.java
@@ -0,0 +1,40 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.common.compress;
+
+/**
+ * Supported compression types
+ *
+ * @opensearch.internal
+ */
+public enum CompressorType {
+
+    DEFLATE {
+        @Override
+        public Compressor compressor() {
+            return CompressorFactory.DEFLATE_COMPRESSOR;
+        }
+    },
+
+    ZSTD {
+        @Override
+        public Compressor compressor() {
+            return CompressorFactory.ZSTD_COMPRESSOR;
+        }
+    },
+
+    NONE {
+        @Override
+        public Compressor compressor() {
+            return CompressorFactory.NONE_COMPRESSOR;
+        }
+    };
+
+    public abstract Compressor compressor();
+}
diff --git a/server/src/main/java/org/opensearch/common/compress/DeflateCompressor.java b/server/src/main/java/org/opensearch/common/compress/DeflateCompressor.java
@@ -157,16 +157,9 @@ public InputStream threadLocalInputStream(InputStream in) throws IOException {
      * @return             decompressing stream
      */
     public static InputStream inputStream(InputStream in, boolean threadLocal) throws IOException {
-        final byte[] headerBytes = new byte[HEADER.length];
-        int len = 0;
-        while (len < headerBytes.length) {
-            final int read = in.read(headerBytes, len, headerBytes.length - len);
-            if (read == -1) {
-                break;
-            }
-            len += read;
-        }
-        if (len != HEADER.length || Arrays.equals(headerBytes, HEADER) == false) {
+        final byte[] header = in.readNBytes(HEADER.length);
+
+        if (Arrays.equals(header, HEADER) == false) {
             throw new IllegalArgumentException("Input stream is not compressed with DEFLATE!");
         }
 
@@ -252,9 +245,11 @@ public BytesReference uncompress(BytesReference bytesReference) throws IOExcepti
         } finally {
             inflater.reset();
         }
-        final BytesReference res = buffer.copyBytes();
-        buffer.reset();
-        return res;
+        try {
+            return buffer.copyBytes();
+        } finally {
+            buffer.reset();
+        }
     }
 
     // Reusable Deflater reference. Note: This is a separate instance from the one used for the compressing stream wrapper because we
@@ -271,8 +266,10 @@ public BytesReference compress(BytesReference bytesReference) throws IOException
         } finally {
             deflater.reset();
         }
-        final BytesReference res = buffer.copyBytes();
-        buffer.reset();
-        return res;
+        try {
+            return buffer.copyBytes();
+        } finally {
+            buffer.reset();
+        }
     }
 }
diff --git a/server/src/main/java/org/opensearch/common/compress/NoneCompressor.java b/server/src/main/java/org/opensearch/common/compress/NoneCompressor.java
@@ -0,0 +1,53 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.common.compress;
+
+import org.opensearch.common.bytes.BytesReference;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+
+/**
+ * {@link Compressor} no compressor implementation.
+ *
+ * @opensearch.internal
+ */
+public class NoneCompressor implements Compressor {
+    @Override
+    public boolean isCompressed(BytesReference bytes) {
+        return false;
+    }
+
+    @Override
+    public int headerLength() {
+        return 0;
+    }
+
+    @Override
+    public InputStream threadLocalInputStream(InputStream in) throws IOException {
+        return in;
+    }
+
+    @Override
+    public OutputStream threadLocalOutputStream(OutputStream out) throws IOException {
+        return out;
+    }
+
+    @Override
+    public BytesReference uncompress(BytesReference bytesReference) throws IOException {
+        return bytesReference;
+    }
+
+    @Override
+    public BytesReference compress(BytesReference bytesReference) throws IOException {
+        return bytesReference;
+    }
+
+}