From 7da82b3efe4d7a4491981869aa2b277191484427 Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Thu, 27 Jul 2023 08:45:27 +0530 Subject: [PATCH] setting up codec settings interface for validation Signed-off-by: Sarthak Aggarwal --- CHANGELOG.md | 2 +- .../opensearch/index/codec/CodecService.java | 12 +-------- .../opensearch/index/codec/CodecSettings.java | 21 +++++++++++++++ .../index/codec/customcodecs/ZstdCodec.java | 10 ++++++- .../codec/customcodecs/ZstdNoDictCodec.java | 10 ++++++- .../opensearch/index/engine/EngineConfig.java | 27 ++++++++++++------- 6 files changed, 59 insertions(+), 23 deletions(-) create mode 100644 server/src/main/java/org/opensearch/index/codec/CodecSettings.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 25516b3771393..95857cfdea188 100644 --- 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 index f847bd82161d2..56360e9df622c 100644 --- a/server/src/main/java/org/opensearch/index/codec/CodecService.java +++ b/server/src/main/java/org/opensearch/index/codec/CodecService.java @@ -39,7 +39,6 @@ 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; @@ -47,7 +46,6 @@ 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 codecs = MapBuilder.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)); @@ -111,8 +105,4 @@ 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 new file mode 100644 index 0000000000000..2d371dfc190db --- /dev/null +++ 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 index 04c110fceacdf..042f7eaa29e53 100644 --- 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 zstd-jni 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 index 134f9a14422ad..a7e8e0e42ee68 100644 --- 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 index 74e251e9eb036..026c983b09bdc 100644 --- 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."); } /**