Browse Source

Fix logging of cluster state update descriptions (#34182)

In #28941 we changed the computation of cluster state task descriptions but
this introduced a bug in which we only log the empty descriptions (rather than
the non-empty ones). This change fixes that.
David Turner 7 years ago
parent
commit
a7ce4b31ed

+ 1 - 1
server/src/main/java/org/elasticsearch/cluster/ClusterStateTaskExecutor.java

@@ -58,7 +58,7 @@ public interface ClusterStateTaskExecutor<T> {
      * This allows groupd task description but the submitting source.
      */
     default String describeTasks(List<T> tasks) {
-        return String.join(", ", tasks.stream().map(t -> (CharSequence)t.toString()).filter(t -> t.length() == 0)::iterator);
+        return String.join(", ", tasks.stream().map(t -> (CharSequence)t.toString()).filter(t -> t.length() > 0)::iterator);
     }
 
     /**

+ 62 - 0
server/src/test/java/org/elasticsearch/cluster/ClusterStateTaskExecutorTests.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.cluster;
+
+import org.elasticsearch.test.ESTestCase;
+
+import java.util.Arrays;
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.equalTo;
+
+public class ClusterStateTaskExecutorTests extends ESTestCase {
+
+    private class TestTask {
+        private final String description;
+
+        TestTask(String description) {
+            this.description = description;
+        }
+
+        @Override
+        public String toString() {
+            return description == null ? "" : "Task{" + description + "}";
+        }
+    }
+
+    public void testDescribeTasks() {
+        final ClusterStateTaskExecutor<TestTask> executor = (currentState, tasks) -> {
+            throw new AssertionError("should not be called");
+        };
+
+        assertThat("describes an empty list", executor.describeTasks(Collections.emptyList()), equalTo(""));
+        assertThat("describes a singleton list", executor.describeTasks(Collections.singletonList(new TestTask("a task"))),
+            equalTo("Task{a task}"));
+        assertThat("describes a list of two tasks",
+            executor.describeTasks(Arrays.asList(new TestTask("a task"), new TestTask("another task"))),
+            equalTo("Task{a task}, Task{another task}"));
+
+        assertThat("skips the only item if it has no description", executor.describeTasks(Collections.singletonList(new TestTask(null))),
+            equalTo(""));
+        assertThat("skips an item if it has no description",
+            executor.describeTasks(Arrays.asList(
+                new TestTask("a task"), new TestTask(null), new TestTask("another task"), new TestTask(null))),
+            equalTo("Task{a task}, Task{another task}"));
+    }
+}