From 55a52a838d89f4a9db99996faf4401a0bf812c17 Mon Sep 17 00:00:00 2001 From: Naveen Tatikonda Date: Mon, 5 Dec 2022 14:50:33 -0600 Subject: [PATCH] [Backport 1.3]Get rid of Rachet to fix failing build (#649) * Get rid of Rachet to fix failing build Signed-off-by: Naveen * Apply spotless on other files in project Signed-off-by: Naveen Signed-off-by: Naveen --- gradle/formatting.gradle | 3 +- .../org/opensearch/knn/index/SpaceType.java | 146 +++++++++--------- .../action/RestLegacyKNNStatsHandlerIT.java | 124 ++++++++------- .../action/RestLegacyKNNWarmupHandlerIT.java | 6 +- 4 files changed, 150 insertions(+), 129 deletions(-) diff --git a/gradle/formatting.gradle b/gradle/formatting.gradle index ec33249c3..837622a54 100644 --- a/gradle/formatting.gradle +++ b/gradle/formatting.gradle @@ -7,8 +7,7 @@ allprojects { project.apply plugin: "com.diffplug.spotless" spotless { java { - ratchetFrom 'origin/1.x' - target '**/*.java' + target 'src/**/*.java' removeUnusedImports() eclipse().configFile rootProject.file('buildSrc/formatterConfig.xml') trimTrailingWhitespace() diff --git a/src/main/java/org/opensearch/knn/index/SpaceType.java b/src/main/java/org/opensearch/knn/index/SpaceType.java index 13e402e3e..ed96cce03 100644 --- a/src/main/java/org/opensearch/knn/index/SpaceType.java +++ b/src/main/java/org/opensearch/knn/index/SpaceType.java @@ -21,86 +21,88 @@ * nmslib calls the inner_product space "negdotprod". This translation should take place in the nmslib's jni layer. */ public enum SpaceType { - L2("l2") { - @Override - public float scoreTranslation(float rawScore) { - return 1 / (1 + rawScore); - } - }, - COSINESIMIL("cosinesimil") { - @Override - public float scoreTranslation(float rawScore) { - return 1 / (1 + rawScore); - } - }, - L1("l1") { - @Override - public float scoreTranslation(float rawScore) { - return 1 / (1 + rawScore); - } - }, - LINF("linf") { - @Override - public float scoreTranslation(float rawScore) { - return 1 / (1 + rawScore); - } - }, - INNER_PRODUCT("innerproduct") { - /** - * The inner product has a range of [-Float.MAX_VALUE, Float.MAX_VALUE], with a more similar result being - * represented by a more negative value. In Lucene, scores have to be in the range of [0, Float.MAX_VALUE], - * where a higher score represents a more similar result. So, we convert here. - * - * @param rawScore score returned from underlying library - * @return Lucene scaled score - */ - @Override - public float scoreTranslation(float rawScore) { - if (rawScore >= 0) { - return 1 / (1 + rawScore); - } - return -rawScore + 1; - } - }, - HAMMING_BIT("hammingbit") { - @Override - public float scoreTranslation(float rawScore) { - return 1 / (1 + rawScore); - } - }; + L2("l2") { + @Override + public float scoreTranslation(float rawScore) { + return 1 / (1 + rawScore); + } + }, + COSINESIMIL("cosinesimil") { + @Override + public float scoreTranslation(float rawScore) { + return 1 / (1 + rawScore); + } + }, + L1("l1") { + @Override + public float scoreTranslation(float rawScore) { + return 1 / (1 + rawScore); + } + }, + LINF("linf") { + @Override + public float scoreTranslation(float rawScore) { + return 1 / (1 + rawScore); + } + }, + INNER_PRODUCT("innerproduct") { + /** + * The inner product has a range of [-Float.MAX_VALUE, Float.MAX_VALUE], with a more similar result being + * represented by a more negative value. In Lucene, scores have to be in the range of [0, Float.MAX_VALUE], + * where a higher score represents a more similar result. So, we convert here. + * + * @param rawScore score returned from underlying library + * @return Lucene scaled score + */ + @Override + public float scoreTranslation(float rawScore) { + if (rawScore >= 0) { + return 1 / (1 + rawScore); + } + return -rawScore + 1; + } + }, + HAMMING_BIT("hammingbit") { + @Override + public float scoreTranslation(float rawScore) { + return 1 / (1 + rawScore); + } + }; - public static SpaceType DEFAULT = L2; + public static SpaceType DEFAULT = L2; - private final String value; + private final String value; - SpaceType(String value) { - this.value = value; - } + SpaceType(String value) { + this.value = value; + } - public abstract float scoreTranslation(float rawScore); + public abstract float scoreTranslation(float rawScore); - /** - * Get space type name in engine - * - * @return name - */ - public String getValue() { return value; } + /** + * Get space type name in engine + * + * @return name + */ + public String getValue() { + return value; + } - public static Set getValues() { - Set values = new HashSet<>(); + public static Set getValues() { + Set values = new HashSet<>(); - for (SpaceType spaceType : SpaceType.values()) { - values.add(spaceType.getValue()); + for (SpaceType spaceType : SpaceType.values()) { + values.add(spaceType.getValue()); + } + return values; } - return values; - } - public static SpaceType getSpace(String spaceTypeName) { - for (SpaceType currentSpaceType : SpaceType.values()) { - if (currentSpaceType.getValue().equalsIgnoreCase(spaceTypeName)) { - return currentSpaceType; - } + public static SpaceType getSpace(String spaceTypeName) { + for (SpaceType currentSpaceType : SpaceType.values()) { + if (currentSpaceType.getValue().equalsIgnoreCase(spaceTypeName)) { + return currentSpaceType; + } + } + throw new IllegalArgumentException("Unable to find space: " + spaceTypeName); } - throw new IllegalArgumentException("Unable to find space: " + spaceTypeName); - } } diff --git a/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java b/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java index 31a6f92b7..e4fe1891f 100644 --- a/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java +++ b/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNStatsHandlerIT.java @@ -87,11 +87,11 @@ public void testStatsValueCheck() throws IOException { createKnnIndex(INDEX_NAME, createKnnIndexMapping(FIELD_NAME, 2)); // Index test document - Float[] vector = {6.0f, 6.0f}; + Float[] vector = { 6.0f, 6.0f }; addKnnDoc(INDEX_NAME, "1", FIELD_NAME, vector); // First search: Ensure that misses=1 - float[] qvector = {6.0f, 6.0f}; + float[] qvector = { 6.0f, 6.0f }; searchKNNIndex(INDEX_NAME, new KNNQueryBuilder(FIELD_NAME, qvector, 1), 1); response = executeKnnStatRequest(Collections.emptyList(), Collections.emptyList(), KNNPlugin.LEGACY_KNN_BASE_URI); @@ -142,8 +142,10 @@ public void testValidMetricsStats() throws IOException { * Test checks that handler correctly returns failure on an invalid metric */ public void testInvalidMetricsStats() { - expectThrows(ResponseException.class, () -> executeKnnStatRequest(Collections.emptyList(), - Collections.singletonList("invalid_metric"), KNNPlugin.LEGACY_KNN_BASE_URI)); + expectThrows( + ResponseException.class, + () -> executeKnnStatRequest(Collections.emptyList(), Collections.singletonList("invalid_metric"), KNNPlugin.LEGACY_KNN_BASE_URI) + ); } /** @@ -152,7 +154,11 @@ public void testInvalidMetricsStats() { * @throws IOException throws IOException */ public void testValidNodeIdStats() throws IOException { - Response response = executeKnnStatRequest(Collections.singletonList("_local"), Collections.emptyList(), KNNPlugin.LEGACY_KNN_BASE_URI); + Response response = executeKnnStatRequest( + Collections.singletonList("_local"), + Collections.emptyList(), + KNNPlugin.LEGACY_KNN_BASE_URI + ); String responseBody = EntityUtils.toString(response.getEntity()); List> nodeStats = parseNodeStatsResponse(responseBody); assertEquals(1, nodeStats.size()); @@ -164,7 +170,11 @@ public void testValidNodeIdStats() throws IOException { * @throws Exception throws Exception */ public void testInvalidNodeIdStats() throws Exception { - Response response = executeKnnStatRequest(Collections.singletonList("invalid_node"), Collections.emptyList(), KNNPlugin.LEGACY_KNN_BASE_URI); + Response response = executeKnnStatRequest( + Collections.singletonList("invalid_node"), + Collections.emptyList(), + KNNPlugin.LEGACY_KNN_BASE_URI + ); String responseBody = EntityUtils.toString(response.getEntity()); List> nodeStats = parseNodeStatsResponse(responseBody); assertEquals(0, nodeStats.size()); @@ -177,11 +187,15 @@ public void testScriptStats_singleShard() throws Exception { clearScriptCache(); // Get initial stats - Response response = executeKnnStatRequest(Collections.emptyList(), Arrays.asList( - StatNames.SCRIPT_COMPILATIONS.getName(), - StatNames.SCRIPT_QUERY_REQUESTS.getName(), - StatNames.SCRIPT_QUERY_ERRORS.getName()) - , KNNPlugin.LEGACY_KNN_BASE_URI); + Response response = executeKnnStatRequest( + Collections.emptyList(), + Arrays.asList( + StatNames.SCRIPT_COMPILATIONS.getName(), + StatNames.SCRIPT_QUERY_REQUESTS.getName(), + StatNames.SCRIPT_QUERY_ERRORS.getName() + ), + KNNPlugin.LEGACY_KNN_BASE_URI + ); List> nodeStats = parseNodeStatsResponse(EntityUtils.toString(response.getEntity())); int initialScriptCompilations = (int) (nodeStats.get(0).get(StatNames.SCRIPT_COMPILATIONS.getName())); int initialScriptQueryRequests = (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_REQUESTS.getName())); @@ -189,28 +203,28 @@ public void testScriptStats_singleShard() throws Exception { // Create an index with a single vector createKnnIndex(INDEX_NAME, createKnnIndexMapping(FIELD_NAME, 2)); - Float[] vector = {6.0f, 6.0f}; + Float[] vector = { 6.0f, 6.0f }; addKnnDoc(INDEX_NAME, "1", FIELD_NAME, vector); // Check l2 query and script compilation stats QueryBuilder qb = new MatchAllQueryBuilder(); Map params = new HashMap<>(); - float[] queryVector = {1.0f, 1.0f}; + float[] queryVector = { 1.0f, 1.0f }; params.put("field", FIELD_NAME); params.put("query_value", queryVector); params.put("space_type", SpaceType.L2.getValue()); Request request = constructKNNScriptQueryRequest(INDEX_NAME, qb, params); response = client().performRequest(request); - assertEquals(request.getEndpoint() + ": failed", RestStatus.OK, - RestStatus.fromCode(response.getStatusLine().getStatusCode())); + assertEquals(request.getEndpoint() + ": failed", RestStatus.OK, RestStatus.fromCode(response.getStatusLine().getStatusCode())); - response = executeKnnStatRequest(Collections.emptyList(), Arrays.asList( - StatNames.SCRIPT_COMPILATIONS.getName(), - StatNames.SCRIPT_QUERY_REQUESTS.getName()), KNNPlugin.LEGACY_KNN_BASE_URI); + response = executeKnnStatRequest( + Collections.emptyList(), + Arrays.asList(StatNames.SCRIPT_COMPILATIONS.getName(), StatNames.SCRIPT_QUERY_REQUESTS.getName()), + KNNPlugin.LEGACY_KNN_BASE_URI + ); nodeStats = parseNodeStatsResponse(EntityUtils.toString(response.getEntity())); assertEquals((int) (nodeStats.get(0).get(StatNames.SCRIPT_COMPILATIONS.getName())), initialScriptCompilations + 1); - assertEquals(initialScriptQueryRequests + 1, - (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_REQUESTS.getName()))); + assertEquals(initialScriptQueryRequests + 1, (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_REQUESTS.getName()))); // Check query error stats params = new HashMap<>(); @@ -221,11 +235,13 @@ public void testScriptStats_singleShard() throws Exception { Request finalRequest = request; expectThrows(ResponseException.class, () -> client().performRequest(finalRequest)); - response = executeKnnStatRequest(Collections.emptyList(), Collections.singletonList( - StatNames.SCRIPT_QUERY_ERRORS.getName()), KNNPlugin.LEGACY_KNN_BASE_URI); + response = executeKnnStatRequest( + Collections.emptyList(), + Collections.singletonList(StatNames.SCRIPT_QUERY_ERRORS.getName()), + KNNPlugin.LEGACY_KNN_BASE_URI + ); nodeStats = parseNodeStatsResponse(EntityUtils.toString(response.getEntity())); - assertEquals(initialScriptQueryErrors + 1, - (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_ERRORS.getName()))); + assertEquals(initialScriptQueryErrors + 1, (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_ERRORS.getName()))); } /** @@ -235,24 +251,28 @@ public void testScriptStats_multipleShards() throws Exception { clearScriptCache(); // Get initial stats - Response response = executeKnnStatRequest(Collections.emptyList(), Arrays.asList( - StatNames.SCRIPT_COMPILATIONS.getName(), - StatNames.SCRIPT_QUERY_REQUESTS.getName(), - StatNames.SCRIPT_QUERY_ERRORS.getName()), KNNPlugin.LEGACY_KNN_BASE_URI); + Response response = executeKnnStatRequest( + Collections.emptyList(), + Arrays.asList( + StatNames.SCRIPT_COMPILATIONS.getName(), + StatNames.SCRIPT_QUERY_REQUESTS.getName(), + StatNames.SCRIPT_QUERY_ERRORS.getName() + ), + KNNPlugin.LEGACY_KNN_BASE_URI + ); List> nodeStats = parseNodeStatsResponse(EntityUtils.toString(response.getEntity())); int initialScriptCompilations = (int) (nodeStats.get(0).get(StatNames.SCRIPT_COMPILATIONS.getName())); int initialScriptQueryRequests = (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_REQUESTS.getName())); int initialScriptQueryErrors = (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_ERRORS.getName())); // Create an index with a single vector - createKnnIndex(INDEX_NAME, Settings.builder() - .put("number_of_shards", 2) - .put("number_of_replicas", 0) - .put("index.knn", true) - .build(), - createKnnIndexMapping(FIELD_NAME, 2)); - - Float[] vector = {6.0f, 6.0f}; + createKnnIndex( + INDEX_NAME, + Settings.builder().put("number_of_shards", 2).put("number_of_replicas", 0).put("index.knn", true).build(), + createKnnIndexMapping(FIELD_NAME, 2) + ); + + Float[] vector = { 6.0f, 6.0f }; addKnnDoc(INDEX_NAME, "1", FIELD_NAME, vector); addKnnDoc(INDEX_NAME, "2", FIELD_NAME, vector); addKnnDoc(INDEX_NAME, "3", FIELD_NAME, vector); @@ -261,24 +281,24 @@ public void testScriptStats_multipleShards() throws Exception { // Check l2 query and script compilation stats QueryBuilder qb = new MatchAllQueryBuilder(); Map params = new HashMap<>(); - float[] queryVector = {1.0f, 1.0f}; + float[] queryVector = { 1.0f, 1.0f }; params.put("field", FIELD_NAME); params.put("query_value", queryVector); params.put("space_type", SpaceType.L2.getValue()); Request request = constructKNNScriptQueryRequest(INDEX_NAME, qb, params); response = client().performRequest(request); - assertEquals(request.getEndpoint() + ": failed", RestStatus.OK, - RestStatus.fromCode(response.getStatusLine().getStatusCode())); + assertEquals(request.getEndpoint() + ": failed", RestStatus.OK, RestStatus.fromCode(response.getStatusLine().getStatusCode())); - response = executeKnnStatRequest(Collections.emptyList(), Arrays.asList( - StatNames.SCRIPT_COMPILATIONS.getName(), - StatNames.SCRIPT_QUERY_REQUESTS.getName()), KNNPlugin.LEGACY_KNN_BASE_URI); + response = executeKnnStatRequest( + Collections.emptyList(), + Arrays.asList(StatNames.SCRIPT_COMPILATIONS.getName(), StatNames.SCRIPT_QUERY_REQUESTS.getName()), + KNNPlugin.LEGACY_KNN_BASE_URI + ); nodeStats = parseNodeStatsResponse(EntityUtils.toString(response.getEntity())); assertEquals((int) (nodeStats.get(0).get(StatNames.SCRIPT_COMPILATIONS.getName())), initialScriptCompilations + 1); - //TODO fix the test case. For some reason request count is treated as 4. + // TODO fix the test case. For some reason request count is treated as 4. // https://github.com/opendistro-for-elasticsearch/k-NN/issues/272 - assertEquals(initialScriptQueryRequests + 4, - (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_REQUESTS.getName()))); + assertEquals(initialScriptQueryRequests + 4, (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_REQUESTS.getName()))); // Check query error stats params = new HashMap<>(); @@ -289,20 +309,20 @@ public void testScriptStats_multipleShards() throws Exception { Request finalRequest = request; expectThrows(ResponseException.class, () -> client().performRequest(finalRequest)); - response = executeKnnStatRequest(Collections.emptyList(), Collections.singletonList( - StatNames.SCRIPT_QUERY_ERRORS.getName()), KNNPlugin.LEGACY_KNN_BASE_URI); + response = executeKnnStatRequest( + Collections.emptyList(), + Collections.singletonList(StatNames.SCRIPT_QUERY_ERRORS.getName()), + KNNPlugin.LEGACY_KNN_BASE_URI + ); nodeStats = parseNodeStatsResponse(EntityUtils.toString(response.getEntity())); - assertEquals(initialScriptQueryErrors + 2, - (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_ERRORS.getName()))); + assertEquals(initialScriptQueryErrors + 2, (int) (nodeStats.get(0).get(StatNames.SCRIPT_QUERY_ERRORS.getName()))); } // Useful settings when debugging to prevent timeouts @Override protected Settings restClientSettings() { if (isDebuggingTest || isDebuggingRemoteCluster) { - return Settings.builder() - .put(CLIENT_SOCKET_TIMEOUT, TimeValue.timeValueMinutes(10)) - .build(); + return Settings.builder().put(CLIENT_SOCKET_TIMEOUT, TimeValue.timeValueMinutes(10)).build(); } else { return super.restClientSettings(); } diff --git a/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNWarmupHandlerIT.java b/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNWarmupHandlerIT.java index 93caf74ad..f74135c0c 100644 --- a/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNWarmupHandlerIT.java +++ b/src/test/java/org/opensearch/knn/plugin/action/RestLegacyKNNWarmupHandlerIT.java @@ -55,7 +55,7 @@ public void testEmptyIndex() throws IOException { public void testSingleIndex() throws IOException { int graphCountBefore = getTotalGraphsInCache(); createKnnIndex(testIndexName, getKNNDefaultIndexSettings(), createKnnIndexMapping(testFieldName, dimensions)); - addKnnDoc(testIndexName, "1", testFieldName, new Float[]{6.0f, 6.0f}); + addKnnDoc(testIndexName, "1", testFieldName, new Float[] { 6.0f, 6.0f }); executeWarmupRequest(Collections.singletonList(testIndexName), KNNPlugin.LEGACY_KNN_BASE_URI); @@ -66,10 +66,10 @@ public void testMultipleIndices() throws IOException { int graphCountBefore = getTotalGraphsInCache(); createKnnIndex(testIndexName + "1", getKNNDefaultIndexSettings(), createKnnIndexMapping(testFieldName, dimensions)); - addKnnDoc(testIndexName + "1", "1", testFieldName, new Float[]{6.0f, 6.0f}); + addKnnDoc(testIndexName + "1", "1", testFieldName, new Float[] { 6.0f, 6.0f }); createKnnIndex(testIndexName + "2", getKNNDefaultIndexSettings(), createKnnIndexMapping(testFieldName, dimensions)); - addKnnDoc(testIndexName + "2", "1", testFieldName, new Float[]{6.0f, 6.0f}); + addKnnDoc(testIndexName + "2", "1", testFieldName, new Float[] { 6.0f, 6.0f }); executeWarmupRequest(Arrays.asList(testIndexName + "1", testIndexName + "2"), KNNPlugin.LEGACY_KNN_BASE_URI);