Merge branch 'main' into bug/weighted-routing-metadata

opensearch-project · Oct 7, 2022 · 9025518 · 9025518
2 parents 9e22066 + f5ea1a4
commit 9025518
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -117,8 +117,11 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Segment Replication] Adding check to make sure checkpoint is not processed when a shard's shard routing is primary ([#4630](https://github.com/opensearch-project/OpenSearch/pull/4630))
 - [Bug]: Fixed invalid location of JDK dependency for arm64 architecture([#4613](https://github.com/opensearch-project/OpenSearch/pull/4613))
 - [Bug]: Alias filter lost after rollover ([#4499](https://github.com/opensearch-project/OpenSearch/pull/4499))
+- Fixed misunderstanding message "No OpenSearchException found" when detailed_error disabled ([#4669](https://github.com/opensearch-project/OpenSearch/pull/4669))
 - Attempt to fix Github workflow for Gradle Check job ([#4679](https://github.com/opensearch-project/OpenSearch/pull/4679))
 - Fix flaky DecommissionControllerTests.testTimesOut ([4683](https://github.com/opensearch-project/OpenSearch/pull/4683))
+- Fix new race condition in DecommissionControllerTests ([4688](https://github.com/opensearch-project/OpenSearch/pull/4688))
+- Fix SearchStats (de)serialization (caused by https://github.com/opensearch-project/OpenSearch/pull/4616) ([#4697](https://github.com/opensearch-project/OpenSearch/pull/4697))
 
 ### Security
 

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.shards/10_basic.yml
@@ -1,14 +1,14 @@
 ---
 "Help":
   - skip:
-      version: " - 2.9.99"
-      reason:  point in time stats were added in 3.0.0
+      version: " - 2.3.99"
+      reason:  point in time stats were added in 2.4.0
       features: node_selector
   - do:
       cat.shards:
         help: true
       node_selector:
-        version: "3.0.0 - "
+        version: "2.4.0 - "
 
   - match:
       $body: |
@@ -88,16 +88,16 @@
                     path.state                       .+   \n
                $/
 ---
-"Help before - 3.0.0":
+"Help before - 2.4.0":
   - skip:
-      version: "3.0.0 - "
-      reason:  point in time stats were added in 3.0.0
+      version: "2.4.0 - "
+      reason:  point in time stats were added in 2.4.0
       features: node_selector
   - do:
       cat.shards:
         help: true
       node_selector:
-        version: " - 2.9.99"
+        version: " - 2.3.99"
 
   - match:
       $body: |

diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/pit/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/pit/10_basic.yml
@@ -1,7 +1,7 @@
 "Create PIT, Search with PIT ID and Delete":
   - skip:
-      version: " - 2.9.99"
-      reason: "mode to be introduced later than 3.0"
+      version: " - 2.3.99"
+      reason: "mode to be introduced later than 2.4.0"
   - do:
       indices.create:
         index: test_pit

diff --git a/server/src/main/java/org/opensearch/ExceptionsHelper.java b/server/src/main/java/org/opensearch/ExceptionsHelper.java
@@ -99,6 +99,21 @@ public static RestStatus status(Throwable t) {
         return RestStatus.INTERNAL_SERVER_ERROR;
     }
 
+    public static String summaryMessage(Throwable t) {
+        if (t != null) {
+            if (t instanceof OpenSearchException) {
+                return t.getClass().getSimpleName() + "[" + t.getMessage() + "]";
+            } else if (t instanceof IllegalArgumentException) {
+                return "Invalid argument";
+            } else if (t instanceof JsonParseException) {
+                return "Failed to parse JSON";
+            } else if (t instanceof OpenSearchRejectedExecutionException) {
+                return "Too many requests";
+            }
+        }
+        return "Internal failure";
+    }
+
     public static Throwable unwrapCause(Throwable t) {
         int counter = 0;
         Throwable result = t;

diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/server/src/main/java/org/opensearch/OpenSearchException.java
@@ -594,16 +594,14 @@ public static void generateFailureXContent(XContentBuilder builder, Params param
 
         // Render the exception with a simple message
         if (detailed == false) {
-            String message = "No OpenSearchException found";
             Throwable t = e;
             for (int counter = 0; counter < 10 && t != null; counter++) {
                 if (t instanceof OpenSearchException) {
-                    message = t.getClass().getSimpleName() + "[" + t.getMessage() + "]";
                     break;
                 }
                 t = t.getCause();
             }
-            builder.field(ERROR, message);
+            builder.field(ERROR, ExceptionsHelper.summaryMessage(t));
             return;
         }
 

diff --git a/server/src/main/java/org/opensearch/index/search/stats/SearchStats.java b/server/src/main/java/org/opensearch/index/search/stats/SearchStats.java
@@ -141,7 +141,7 @@ private Stats(StreamInput in) throws IOException {
             suggestTimeInMillis = in.readVLong();
             suggestCurrent = in.readVLong();
 
-            if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
+            if (in.getVersion().onOrAfter(Version.V_2_4_0)) {
                 pitCount = in.readVLong();
                 pitTimeInMillis = in.readVLong();
                 pitCurrent = in.readVLong();
@@ -292,7 +292,7 @@ public void writeTo(StreamOutput out) throws IOException {
             out.writeVLong(suggestTimeInMillis);
             out.writeVLong(suggestCurrent);
 
-            if (out.getVersion().onOrAfter(Version.V_3_0_0)) {
+            if (out.getVersion().onOrAfter(Version.V_2_4_0)) {
                 out.writeVLong(pitCount);
                 out.writeVLong(pitTimeInMillis);
                 out.writeVLong(pitCurrent);

diff --git a/server/src/test/java/org/opensearch/ExceptionsHelperTests.java b/server/src/test/java/org/opensearch/ExceptionsHelperTests.java
@@ -113,6 +113,12 @@ public void testStatus() {
         assertThat(ExceptionsHelper.status(new OpenSearchRejectedExecutionException("rejected")), equalTo(RestStatus.TOO_MANY_REQUESTS));
     }
 
+    public void testSummaryMessage() {
+        assertThat(ExceptionsHelper.summaryMessage(new IllegalArgumentException("illegal")), equalTo("Invalid argument"));
+        assertThat(ExceptionsHelper.summaryMessage(new JsonParseException(null, "illegal")), equalTo("Failed to parse JSON"));
+        assertThat(ExceptionsHelper.summaryMessage(new OpenSearchRejectedExecutionException("rejected")), equalTo("Too many requests"));
+    }
+
     public void testGroupBy() {
         ShardOperationFailedException[] failures = new ShardOperationFailedException[] {
             createShardFailureParsingException("error", "node0", "index", 0, null),

diff --git a/server/src/test/java/org/opensearch/OpenSearchExceptionTests.java b/server/src/test/java/org/opensearch/OpenSearchExceptionTests.java
@@ -814,12 +814,7 @@ public void testFailureToAndFromXContentWithNoDetails() throws IOException {
         }
         assertNotNull(parsedFailure);
 
-        String reason;
-        if (failure instanceof OpenSearchException) {
-            reason = failure.getClass().getSimpleName() + "[" + failure.getMessage() + "]";
-        } else {
-            reason = "No OpenSearchException found";
-        }
+        String reason = ExceptionsHelper.summaryMessage(failure);
         assertEquals(OpenSearchException.buildMessage("exception", reason, null), parsedFailure.getMessage());
         assertEquals(0, parsedFailure.getHeaders().size());
         assertEquals(0, parsedFailure.getMetadata().size());

diff --git a/server/src/test/java/org/opensearch/cluster/decommission/DecommissionControllerTests.java b/server/src/test/java/org/opensearch/cluster/decommission/DecommissionControllerTests.java
@@ -223,7 +223,9 @@ public void testTimesOut() throws InterruptedException {
             TimeValue.timeValueMillis(0),
             new ActionListener<>() {
                 @Override
-                public void onResponse(Void unused) {}
+                public void onResponse(Void unused) {
+                    countDownLatch.countDown();
+                }
 
                 @Override
                 public void onFailure(Exception e) {
@@ -232,10 +234,10 @@ public void onFailure(Exception e) {
                 }
             }
         );
+        assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
         MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue());
         MatcherAssert.assertThat(exceptionReference.get(), instanceOf(OpenSearchTimeoutException.class));
         MatcherAssert.assertThat(exceptionReference.get().getMessage(), containsString("waiting for removal of decommissioned nodes"));
-        assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
     }
 
     public void testSuccessfulDecommissionStatusMetadataUpdate() throws InterruptedException {

diff --git a/server/src/test/java/org/opensearch/rest/BytesRestResponseTests.java b/server/src/test/java/org/opensearch/rest/BytesRestResponseTests.java
@@ -117,7 +117,7 @@ public void testNonOpenSearchExceptionIsNotShownAsSimpleMessage() throws Excepti
         assertThat(text, not(containsString("UnknownException[an error occurred reading data]")));
         assertThat(text, not(containsString("FileNotFoundException[/foo/bar]")));
         assertThat(text, not(containsString("error_trace")));
-        assertThat(text, containsString("\"error\":\"No OpenSearchException found\""));
+        assertThat(text, containsString("\"error\":\"Internal failure\""));
     }
 
     public void testErrorTrace() throws Exception {