fix: MinHash token filter parameters not working (opensearch-project#…

…15233) * fix: minhash configuration Signed-off-by: Matt Ridehalgh <mrideh@amazon.co.uk> * style: linting Signed-off-by: Matt Ridehalgh <mrideh@amazon.co.uk> * chore: update CHANGELOG.md Signed-off-by: Matt Ridehalgh <mrideh@amazon.co.uk> --------- Signed-off-by: Matt Ridehalgh <mrideh@amazon.co.uk> (cherry picked from commit 9661e8d)
mridehalgh · Aug 19, 2024 · 0c4b729 · 0c4b729
1 parent 6581248
commit 0c4b729
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ## [Unreleased 2.x]
 ### Added
+- [Offline Nodes] Adds offline-tasks library containing various interfaces to be used for Offline Background Tasks. ([#13574](https://github.com/opensearch-project/OpenSearch/pull/13574))
 - Fix for hasInitiatedFetching to fix allocation explain and manual reroute APIs (([#14972](https://github.com/opensearch-project/OpenSearch/pull/14972))
 - [Workload Management] Add queryGroupId to Task ([14708](https://github.com/opensearch-project/OpenSearch/pull/14708))
 - Add setting to ignore throttling nodes for allocation of unassigned primaries in remote restore ([#14991](https://github.com/opensearch-project/OpenSearch/pull/14991))
@@ -22,17 +23,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Dependencies
 - Bump `netty` from 4.1.111.Final to 4.1.112.Final ([#15081](https://github.com/opensearch-project/OpenSearch/pull/15081))
-- Bump `org.apache.commons:commons-lang3` from 3.14.0 to 3.15.0 ([#14861](https://github.com/opensearch-project/OpenSearch/pull/14861))
+- Bump `org.apache.commons:commons-lang3` from 3.14.0 to 3.16.0 ([#14861](https://github.com/opensearch-project/OpenSearch/pull/14861), [#15205](https://github.com/opensearch-project/OpenSearch/pull/15205))
 - OpenJDK Update (July 2024 Patch releases) ([#14998](https://github.com/opensearch-project/OpenSearch/pull/14998))
 - Bump `com.microsoft.azure:msal4j` from 1.16.1 to 1.16.2 ([#14995](https://github.com/opensearch-project/OpenSearch/pull/14995))
 - Bump `actions/github-script` from 6 to 7 ([#14997](https://github.com/opensearch-project/OpenSearch/pull/14997))
 - Bump `org.tukaani:xz` from 1.9 to 1.10 ([#15110](https://github.com/opensearch-project/OpenSearch/pull/15110))
+- Bump `actions/setup-java` from 1 to 4 ([#15104](https://github.com/opensearch-project/OpenSearch/pull/15104))
 - Bump `org.apache.avro:avro` from 1.11.3 to 1.12.0 in /plugins/repository-hdfs ([#15119](https://github.com/opensearch-project/OpenSearch/pull/15119))
 - Bump `org.bouncycastle:bcpg-fips` from 1.0.7.1 to 2.0.8 and `org.bouncycastle:bc-fips` from 1.0.2.5 to 2.0.0 in /distribution/tools/plugin-cli ([#15103](https://github.com/opensearch-project/OpenSearch/pull/15103))
 - Bump `com.azure:azure-core` from 1.49.1 to 1.51.0 ([#15111](https://github.com/opensearch-project/OpenSearch/pull/15111))
 - Bump `org.xerial.snappy:snappy-java` from 1.1.10.5 to 1.1.10.6 ([#15207](https://github.com/opensearch-project/OpenSearch/pull/15207))
 - Bump `com.azure:azure-xml` from 1.0.0 to 1.1.0 ([#15206](https://github.com/opensearch-project/OpenSearch/pull/15206))
-- Bump `reactor` from 3.5.19 to 3.5.20 ([#15262](https://github.com/opensearch-project/OpenSearch/pull/15262)) 
+- Bump `reactor` from 3.5.19 to 3.5.20 ([#15262](https://github.com/opensearch-project/OpenSearch/pull/15262))
 - Bump `reactor-netty` from 1.1.21 to 1.1.22 ([#15262](https://github.com/opensearch-project/OpenSearch/pull/15262))
 
 ### Changed
@@ -49,6 +51,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Fix delete index template failed when the index template matches a data stream but is unused ([#15080](https://github.com/opensearch-project/OpenSearch/pull/15080))
 - Fix array_index_out_of_bounds_exception when indexing documents with field name containing only dot ([#15126](https://github.com/opensearch-project/OpenSearch/pull/15126))
 - Fixed array field name omission in flat_object function for nested JSON ([#13620](https://github.com/opensearch-project/OpenSearch/pull/13620))
+- Fix range aggregation optimization ignoring top level queries ([#15194](https://github.com/opensearch-project/OpenSearch/pull/15194))
+- Fix incorrect parameter names in MinHash token filter configuration handling ([#15233](https://github.com/opensearch-project/OpenSearch/pull/15233))
 
 ### Security
 

diff --git a/...alysis-common/src/main/java/org/opensearch/analysis/common/MinHashTokenFilterFactory.java b/...alysis-common/src/main/java/org/opensearch/analysis/common/MinHashTokenFilterFactory.java
@@ -65,10 +65,10 @@ private Map<String, String> convertSettings(Settings settings) {
         if (settings.hasValue("hash_count")) {
             settingMap.put("hashCount", settings.get("hash_count"));
         }
-        if (settings.hasValue("bucketCount")) {
+        if (settings.hasValue("bucket_count")) {
             settingMap.put("bucketCount", settings.get("bucket_count"));
         }
-        if (settings.hasValue("hashSetSize")) {
+        if (settings.hasValue("hash_set_size")) {
             settingMap.put("hashSetSize", settings.get("hash_set_size"));
         }
         if (settings.hasValue("with_rotation")) {

diff --git a/...alysis-common/src/test/java/org/opensearch/analysis/common/MinHashFilterFactoryTests.java b/...alysis-common/src/test/java/org/opensearch/analysis/common/MinHashFilterFactoryTests.java
@@ -53,8 +53,7 @@ public void testDefault() throws IOException {
         OpenSearchTestCase.TestAnalysis analysis = AnalysisTestsHelper.createTestAnalysisFromSettings(settings, new CommonAnalysisPlugin());
         TokenFilterFactory tokenFilter = analysis.tokenFilter.get("min_hash");
         String source = "the quick brown fox";
-        Tokenizer tokenizer = new WhitespaceTokenizer();
-        tokenizer.setReader(new StringReader(source));
+        Tokenizer tokenizer = getTokenizer(source);
 
         // with_rotation is true by default, and hash_set_size is 1, so even though the source doesn't
         // have enough tokens to fill all the buckets, we still expect 512 tokens.
@@ -73,11 +72,60 @@ public void testSettings() throws IOException {
         OpenSearchTestCase.TestAnalysis analysis = AnalysisTestsHelper.createTestAnalysisFromSettings(settings, new CommonAnalysisPlugin());
         TokenFilterFactory tokenFilter = analysis.tokenFilter.get("test_min_hash");
         String source = "sushi";
-        Tokenizer tokenizer = new WhitespaceTokenizer();
-        tokenizer.setReader(new StringReader(source));
+        Tokenizer tokenizer = getTokenizer(source);
 
         // despite the fact that bucket_count is 2 and hash_set_size is 1,
         // because with_rotation is false, we only expect 1 token here.
         assertStreamHasNumberOfTokens(tokenFilter.create(tokenizer), 1);
     }
+
+    public void testBucketCountSetting() throws IOException {
+        // Correct case with "bucket_count"
+        Settings settingsWithBucketCount = Settings.builder()
+            .put("index.analysis.filter.test_min_hash.type", "min_hash")
+            .put("index.analysis.filter.test_min_hash.hash_count", "1")
+            .put("index.analysis.filter.test_min_hash.bucket_count", "3")
+            .put("index.analysis.filter.test_min_hash.hash_set_size", "1")
+            .put("index.analysis.filter.test_min_hash.with_rotation", false)
+            .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
+            .build();
+
+        OpenSearchTestCase.TestAnalysis analysisWithBucketCount = getTestAnalysisFromSettings(settingsWithBucketCount);
+
+        TokenFilterFactory tokenFilterWithBucketCount = analysisWithBucketCount.tokenFilter.get("test_min_hash");
+        String sourceWithBucketCount = "salmon avocado roll uramaki";
+        Tokenizer tokenizerWithBucketCount = getTokenizer(sourceWithBucketCount);
+        // Expect 3 tokens due to bucket_count being set to 3
+        assertStreamHasNumberOfTokens(tokenFilterWithBucketCount.create(tokenizerWithBucketCount), 3);
+    }
+
+    public void testHashSetSizeSetting() throws IOException {
+        // Correct case with "hash_set_size"
+        Settings settingsWithHashSetSize = Settings.builder()
+            .put("index.analysis.filter.test_min_hash.type", "min_hash")
+            .put("index.analysis.filter.test_min_hash.hash_count", "1")
+            .put("index.analysis.filter.test_min_hash.bucket_count", "1")
+            .put("index.analysis.filter.test_min_hash.hash_set_size", "2")
+            .put("index.analysis.filter.test_min_hash.with_rotation", false)
+            .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
+            .build();
+
+        OpenSearchTestCase.TestAnalysis analysisWithHashSetSize = getTestAnalysisFromSettings(settingsWithHashSetSize);
+
+        TokenFilterFactory tokenFilterWithHashSetSize = analysisWithHashSetSize.tokenFilter.get("test_min_hash");
+        String sourceWithHashSetSize = "salmon avocado roll uramaki";
+        Tokenizer tokenizerWithHashSetSize = getTokenizer(sourceWithHashSetSize);
+        // Expect 2 tokens due to hash_set_size being set to 2 and bucket_count being 1
+        assertStreamHasNumberOfTokens(tokenFilterWithHashSetSize.create(tokenizerWithHashSetSize), 2);
+    }
+
+    private static OpenSearchTestCase.TestAnalysis getTestAnalysisFromSettings(Settings settingsWithBucketCount) throws IOException {
+        return AnalysisTestsHelper.createTestAnalysisFromSettings(settingsWithBucketCount, new CommonAnalysisModulePlugin());
+    }
+
+    private static Tokenizer getTokenizer(String sourceWithBucketCount) {
+        Tokenizer tokenizerWithBucketCount = new WhitespaceTokenizer();
+        tokenizerWithBucketCount.setReader(new StringReader(sourceWithBucketCount));
+        return tokenizerWithBucketCount;
+    }
 }