From 7bc41934cd1fcfe1bc565263c4be1b596fa9383e Mon Sep 17 00:00:00 2001 From: Sarthak Aggarwal Date: Wed, 19 Jul 2023 15:14:39 +0530 Subject: [PATCH] reinstating default and best_compression Signed-off-by: Sarthak Aggarwal --- .../opensearch/index/codec/MultiCodecReindexIT.java | 10 +++++----- .../org/opensearch/index/codec/MultiCodecMergeIT.java | 10 +++++----- .../java/org/opensearch/index/codec/CodecService.java | 6 ++++++ .../java/org/opensearch/index/engine/EngineConfig.java | 7 ++++++- .../common/settings/SettingsModuleTests.java | 5 ++--- .../opensearch/gateway/PrimaryShardAllocatorTests.java | 4 ++-- .../indices/replication/common/CopyStateTests.java | 2 +- 7 files changed, 27 insertions(+), 17 deletions(-) diff --git a/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java b/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java index 3fac7344a9cb4..87f3c68d8af76 100644 --- a/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java +++ b/modules/reindex/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecReindexIT.java @@ -43,13 +43,13 @@ public class MultiCodecReindexIT extends ReindexTestCase { public void testReindexingMultipleCodecs() throws InterruptedException, ExecutionException { internalCluster().ensureAtLeastNumDataNodes(1); Map codecMap = Map.of( - CodecService.ZLIB_CODEC, + "best_compression", "BEST_COMPRESSION", - CodecService.ZSTD_NO_DICT_CODEC, + "zstd_no_dict", "ZSTD_NO_DICT", - CodecService.ZSTD_CODEC, + "zstd", "ZSTD", - CodecService.LZ4_CODEC, + "default", "BEST_SPEED" ); @@ -71,7 +71,7 @@ private void assertReindexingWithMultipleCodecs(String destCodec, String destCod Settings.builder() .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) - .put("index.codec", CodecService.LZ4_CODEC) + .put("index.codec", "default") .put("index.merge.policy.max_merged_segment", "1b") .build() ); diff --git a/server/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java b/server/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java index be70dabf3f4e7..2866292e5e2e0 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/codec/MultiCodecMergeIT.java @@ -43,13 +43,13 @@ public class MultiCodecMergeIT extends OpenSearchIntegTestCase { public void testForceMergeMultipleCodecs() throws ExecutionException, InterruptedException { Map codecMap = Map.of( - CodecService.ZLIB_CODEC, + "best_compression", "BEST_COMPRESSION", - CodecService.ZSTD_NO_DICT_CODEC, + "zstd_no_dict", "ZSTD_NO_DICT", - CodecService.ZSTD_CODEC, + "zstd", "ZSTD", - CodecService.LZ4_CODEC, + "default", "BEST_SPEED" ); @@ -71,7 +71,7 @@ private void forceMergeMultipleCodecs(String finalCodec, String finalCodecMode, Settings.builder() .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) - .put("index.codec", CodecService.LZ4_CODEC) + .put("index.codec", "default") .put("index.merge.policy.max_merged_segment", "1b") .build() ); 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 da64e2954b0d8..a1913d5fa2061 100644 --- a/server/src/main/java/org/opensearch/index/codec/CodecService.java +++ b/server/src/main/java/org/opensearch/index/codec/CodecService.java @@ -60,7 +60,9 @@ public class CodecService { private final Map codecs; public static final String DEFAULT_CODEC = "default"; + public static final String LZ4 = "lz4"; public static final String BEST_COMPRESSION_CODEC = "best_compression"; + public static final String ZLIB = "zlib"; /** * the raw unfiltered lucene default. useful for testing */ @@ -74,12 +76,16 @@ public CodecService(@Nullable MapperService mapperService, IndexSettings indexSe int compressionLevel = indexSettings.getValue(INDEX_CODEC_COMPRESSION_LEVEL_SETTING); if (mapperService == null) { codecs.put(DEFAULT_CODEC, new Lucene95Codec()); + codecs.put(LZ4, new Lucene95Codec()); codecs.put(BEST_COMPRESSION_CODEC, new Lucene95Codec(Mode.BEST_COMPRESSION)); + codecs.put(ZLIB, new Lucene95Codec(Mode.BEST_COMPRESSION)); codecs.put(ZSTD_CODEC, new ZstdCodec(compressionLevel)); codecs.put(ZSTD_NO_DICT_CODEC, new ZstdNoDictCodec(compressionLevel)); } else { codecs.put(DEFAULT_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger)); + codecs.put(LZ4, new PerFieldMappingPostingFormatCodec(Mode.BEST_SPEED, mapperService, logger)); codecs.put(BEST_COMPRESSION_CODEC, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger)); + codecs.put(ZLIB, new PerFieldMappingPostingFormatCodec(Mode.BEST_COMPRESSION, mapperService, logger)); codecs.put(ZSTD_CODEC, new ZstdCodec(mapperService, logger, compressionLevel)); codecs.put(ZSTD_NO_DICT_CODEC, new ZstdNoDictCodec(mapperService, logger, compressionLevel)); } 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 e0aaebf57037a..6647346ff8633 100644 --- a/server/src/main/java/org/opensearch/index/engine/EngineConfig.java +++ b/server/src/main/java/org/opensearch/index/engine/EngineConfig.java @@ -130,6 +130,8 @@ public Supplier retentionLeasesSupplier() { public static final Setting INDEX_CODEC_SETTING = new Setting<>("index.codec", "lz4", s -> { switch (s) { case "lz4": + case "default": + case "best_compression": case "zlib": case "zstd": case "zstd_no_dict": @@ -138,7 +140,8 @@ public Supplier retentionLeasesSupplier() { default: if (Codec.availableCodecs().contains(s) == false) { // we don't error message the not officially supported ones throw new IllegalArgumentException( - "unknown value for [index.codec] must be one of [lz4, zlib, zstd, zstd_no_dict] but was: " + s + "unknown value for [index.codec] must be one of [default, best_compression, lz4, zlib, zstd, zstd_no_dict] but was: " + + s ); } return s; @@ -184,6 +187,8 @@ private static void doValidateCodecSettings(final String codec) { case "best_compression": case "lucene_default": case "default": + case "lz4": + case "zlib": break; default: if (Codec.availableCodecs().contains(codec)) { diff --git a/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java b/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java index 7e8c0967b763e..4490f6b39996f 100644 --- a/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java +++ b/server/src/test/java/org/opensearch/common/settings/SettingsModuleTests.java @@ -37,7 +37,6 @@ import org.hamcrest.Matchers; import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.IndexSettings; -import org.opensearch.index.codec.CodecService; import org.opensearch.search.SearchService; import org.opensearch.test.FeatureFlagSetter; @@ -74,13 +73,13 @@ public void testValidate() { } { - Settings settings = Settings.builder().put("index.codec", CodecService.LZ4_CODEC).put("index.foo.bar", 1).build(); + Settings settings = Settings.builder().put("index.codec", "default").put("index.foo.bar", 1).build(); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> new SettingsModule(settings)); assertEquals("node settings must not contain any index level settings", ex.getMessage()); } { - Settings settings = Settings.builder().put("index.codec", CodecService.LZ4_CODEC).build(); + Settings settings = Settings.builder().put("index.codec", "default").build(); SettingsModule module = new SettingsModule(settings); assertInstanceBinding(module, Settings.class, (s) -> s == settings); } diff --git a/server/src/test/java/org/opensearch/gateway/PrimaryShardAllocatorTests.java b/server/src/test/java/org/opensearch/gateway/PrimaryShardAllocatorTests.java index 7504f6d7852fb..61bf5f347c2d5 100644 --- a/server/src/test/java/org/opensearch/gateway/PrimaryShardAllocatorTests.java +++ b/server/src/test/java/org/opensearch/gateway/PrimaryShardAllocatorTests.java @@ -812,7 +812,7 @@ public TestAllocator addData(DiscoveryNode node, String allocationId, boolean pr node, allocationId, primary, - ReplicationCheckpoint.empty(shardId, new CodecService(null, indexSettings, null).codec(CodecService.LZ4_CODEC).getName()), + ReplicationCheckpoint.empty(shardId, new CodecService(null, indexSettings, null).codec("default").getName()), null ); } @@ -824,7 +824,7 @@ public TestAllocator addData(DiscoveryNode node, String allocationId, boolean pr node, allocationId, primary, - ReplicationCheckpoint.empty(shardId, new CodecService(null, indexSettings, null).codec(CodecService.LZ4_CODEC).getName()), + ReplicationCheckpoint.empty(shardId, new CodecService(null, indexSettings, null).codec("default").getName()), storeException ); } diff --git a/server/src/test/java/org/opensearch/indices/replication/common/CopyStateTests.java b/server/src/test/java/org/opensearch/indices/replication/common/CopyStateTests.java index 463d1a5368781..13ab40203ff2b 100644 --- a/server/src/test/java/org/opensearch/indices/replication/common/CopyStateTests.java +++ b/server/src/test/java/org/opensearch/indices/replication/common/CopyStateTests.java @@ -57,7 +57,7 @@ public void testCopyStateCreation() throws IOException { CopyState copyState = new CopyState( ReplicationCheckpoint.empty( mockIndexShard.shardId(), - new CodecService(null, mockIndexShard.indexSettings(), null).codec(CodecService.LZ4_CODEC).getName() + new CodecService(null, mockIndexShard.indexSettings(), null).codec("default").getName() ), mockIndexShard );