setting up codec settings interface for validation

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
opensearch-project · Jul 28, 2023 · e7bfc17 · e7bfc17
1 parent 3fd0650
commit e7bfc17
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -78,7 +78,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Added
 - Add server version as REST response header [#6583](https://github.com/opensearch-project/OpenSearch/issues/6583)
 - Start replication checkpointTimers on primary before segments upload to remote store. ([#8221]()https://github.com/opensearch-project/OpenSearch/pull/8221)
-- Disallowing compression level to be set for default and best_compression index codecs ([#8737]()https://github.com/opensearch-project/OpenSearch/pull/8737)
+- Disallow compression level to be set for default and best_compression index codecs ([#8737]()https://github.com/opensearch-project/OpenSearch/pull/8737)
 ### Dependencies
 - Bump `org.apache.logging.log4j:log4j-core` from 2.17.1 to 2.20.0 ([#8307](https://github.com/opensearch-project/OpenSearch/pull/8307))
 - Bump `io.grpc:grpc-context` from 1.46.0 to 1.56.1 ([#8726](https://github.com/opensearch-project/OpenSearch/pull/8726))

diff --git a/server/src/main/java/org/opensearch/index/codec/CodecService.java b/server/src/main/java/org/opensearch/index/codec/CodecService.java
@@ -39,15 +39,13 @@
 import org.opensearch.common.Nullable;
 import org.opensearch.common.collect.MapBuilder;
 import org.opensearch.index.IndexSettings;
-import org.opensearch.index.codec.customcodecs.Lucene95CustomCodec;
 import org.opensearch.index.codec.customcodecs.ZstdCodec;
 import org.opensearch.index.codec.customcodecs.ZstdNoDictCodec;
 import org.opensearch.index.mapper.MapperService;
 
 import java.util.Map;
 
 import static org.opensearch.index.engine.EngineConfig.INDEX_CODEC_COMPRESSION_LEVEL_SETTING;
-import static org.opensearch.index.engine.EngineConfig.INDEX_CODEC_SETTING;
 
 /**
  * Since Lucene 4.0 low level index segments are read and written through a
@@ -73,11 +71,7 @@ public class CodecService {
     public CodecService(@Nullable MapperService mapperService, IndexSettings indexSettings, Logger logger) {
         final MapBuilder<String, Codec> codecs = MapBuilder.<String, Codec>newMapBuilder();
         assert null != indexSettings;
-        String codecName = indexSettings.getValue(INDEX_CODEC_SETTING);
-        int compressionLevel = Lucene95CustomCodec.DEFAULT_COMPRESSION_LEVEL;
-        if (isZStandardCodec(codecName)) {
-            compressionLevel = indexSettings.getValue(INDEX_CODEC_COMPRESSION_LEVEL_SETTING);
-        }
+        int compressionLevel = indexSettings.getValue(INDEX_CODEC_COMPRESSION_LEVEL_SETTING);
         if (mapperService == null) {
             codecs.put(DEFAULT_CODEC, new Lucene95Codec());
             codecs.put(BEST_COMPRESSION_CODEC, new Lucene95Codec(Mode.BEST_COMPRESSION));
@@ -110,9 +104,4 @@ public Codec codec(String name) {
     public String[] availableCodecs() {
         return codecs.keySet().toArray(new String[0]);
     }
-
-    public static boolean isZStandardCodec(String codec) {
-        return codec.equals(ZSTD_CODEC) || codec.equals(ZSTD_NO_DICT_CODEC);
-    }
-
 }
diff --git a/server/src/main/java/org/opensearch/index/codec/CodecSettings.java b/server/src/main/java/org/opensearch/index/codec/CodecSettings.java
@@ -0,0 +1,21 @@
+/*
+ * 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.index.codec;
+
+import org.apache.lucene.codecs.Codec;
+import org.opensearch.common.settings.Setting;
+
+/**
+ * This {@link CodecSettings} allows us to manage the settings with {@link Codec}.
+ *
+ * @opensearch.internal
+ */
+public interface CodecSettings {
+    boolean supports(Setting<?> setting);
+}
diff --git a/server/src/main/java/org/opensearch/index/codec/customcodecs/ZstdCodec.java b/server/src/main/java/org/opensearch/index/codec/customcodecs/ZstdCodec.java
@@ -9,12 +9,15 @@
 package org.opensearch.index.codec.customcodecs;
 
 import org.apache.logging.log4j.Logger;
+import org.opensearch.common.settings.Setting;
+import org.opensearch.index.codec.CodecSettings;
+import org.opensearch.index.engine.EngineConfig;
 import org.opensearch.index.mapper.MapperService;
 
 /**
  * ZstdCodec provides ZSTD compressor using the <a href="https://github.com/luben/zstd-jni">zstd-jni</a> library.
  */
-public class ZstdCodec extends Lucene95CustomCodec {
+public class ZstdCodec extends Lucene95CustomCodec implements CodecSettings {
 
     /**
      * Creates a new ZstdCodec instance with the default compression level.
@@ -41,4 +44,9 @@ public ZstdCodec(MapperService mapperService, Logger logger, int compressionLeve
     public String toString() {
         return getClass().getSimpleName();
     }
+
+    @Override
+    public boolean supports(Setting<?> setting) {
+        return setting.equals(EngineConfig.INDEX_CODEC_COMPRESSION_LEVEL_SETTING);
+    }
 }
diff --git a/server/src/main/java/org/opensearch/index/codec/customcodecs/ZstdNoDictCodec.java b/server/src/main/java/org/opensearch/index/codec/customcodecs/ZstdNoDictCodec.java
@@ -9,12 +9,15 @@
 package org.opensearch.index.codec.customcodecs;
 
 import org.apache.logging.log4j.Logger;
+import org.opensearch.common.settings.Setting;
+import org.opensearch.index.codec.CodecSettings;
+import org.opensearch.index.engine.EngineConfig;
 import org.opensearch.index.mapper.MapperService;
 
 /**
  * ZstdNoDictCodec provides ZSTD compressor without a dictionary support.
  */
-public class ZstdNoDictCodec extends Lucene95CustomCodec {
+public class ZstdNoDictCodec extends Lucene95CustomCodec implements CodecSettings {
 
     /**
      * Creates a new ZstdNoDictCodec instance with the default compression level.
@@ -41,4 +44,9 @@ public ZstdNoDictCodec(MapperService mapperService, Logger logger, int compressi
     public String toString() {
         return getClass().getSimpleName();
     }
+
+    @Override
+    public boolean supports(Setting<?> setting) {
+        return setting.equals(EngineConfig.INDEX_CODEC_COMPRESSION_LEVEL_SETTING);
+    }
 }
diff --git a/server/src/main/java/org/opensearch/index/engine/EngineConfig.java b/server/src/main/java/org/opensearch/index/engine/EngineConfig.java
@@ -48,6 +48,7 @@
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.index.codec.CodecService;
+import org.opensearch.index.codec.CodecSettings;
 import org.opensearch.index.mapper.ParsedDocument;
 import org.opensearch.index.seqno.RetentionLeases;
 import org.opensearch.core.index.shard.ShardId;
@@ -68,8 +69,6 @@
 import java.util.function.LongSupplier;
 import java.util.function.Supplier;
 
-import static org.opensearch.index.codec.CodecService.isZStandardCodec;
-
 /**
  * Holds all the configuration that is used to create an {@link Engine}.
  * Once {@link Engine} has been created with this object, changes to this
@@ -178,14 +177,24 @@ public void validate(String key, Object value, Object dependency) {
     };
 
     private static void doValidateCodecSettings(final String codec) {
-        if (!isZStandardCodec(codec)) {
-            throw new IllegalArgumentException(
-                "Compression level cannot be set for the "
-                    + codec
-                    + " codec. Compression level settings is only applicable for zstd and zstd_no_dict codecs."
-            );
+        switch (codec) {
+            case "zstd":
+            case "zstd_no_dict":
+                return;
+            case "best_compression":
+            case "lucene_default":
+            case "default":
+                break;
+            default:
+                if (Codec.availableCodecs().contains(codec)) {
+                    Codec luceneCodec = Codec.forName(codec);
+                    if (luceneCodec instanceof CodecSettings
+                        && ((CodecSettings) luceneCodec).supports(INDEX_CODEC_COMPRESSION_LEVEL_SETTING)) {
+                        return;
+                    }
+                }
         }
-
+        throw new IllegalArgumentException("Compression level cannot be set for the " + codec + " codec.");
     }
 
     /**