From 0fd33b089f3491e9b0b58e6105c17d91dd9b5c7d Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 17 Jul 2019 08:33:16 +0100 Subject: [PATCH] Report shard state changes better (#44419) Today when the cluster health changes the `AllocationService` reports at most ten shards that were started or failed, and always ends its message with `...` suggesting that the list is truncated. This commit adjusts these messages to be clearer about whether the list is truncated or not. When debug logging is enabled the list is not truncated; if the list is truncated then its length is logged, and if it is not truncated then no `...` is included in the message. --- .../routing/allocation/AllocationService.java | 23 +++--- .../allocation/AllocationServiceTests.java | 72 +++++++++++++++++++ 2 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationServiceTests.java diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationService.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationService.java index 9b22e63cf9a..7ba7f0bb578 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationService.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/AllocationService.java @@ -110,8 +110,9 @@ public class AllocationService { applyStartedShards(allocation, startedShards); gatewayAllocator.applyStartedShards(allocation, startedShards); reroute(allocation); - String startedShardsAsString = firstListElementsToCommaDelimitedString(startedShards, s -> s.shardId().toString()); - return buildResultAndLogHealthChange(clusterState, allocation, "shards started [" + startedShardsAsString + "] ..."); + String startedShardsAsString + = firstListElementsToCommaDelimitedString(startedShards, s -> s.shardId().toString(), logger.isDebugEnabled()); + return buildResultAndLogHealthChange(clusterState, allocation, "shards started [" + startedShardsAsString + "]"); } protected ClusterState buildResultAndLogHealthChange(ClusterState oldState, RoutingAllocation allocation, String reason) { @@ -208,8 +209,9 @@ public class AllocationService { gatewayAllocator.applyFailedShards(allocation, failedShards); reroute(allocation); - String failedShardsAsString = firstListElementsToCommaDelimitedString(failedShards, s -> s.getRoutingEntry().shardId().toString()); - return buildResultAndLogHealthChange(clusterState, allocation, "shards failed [" + failedShardsAsString + "] ..."); + String failedShardsAsString + = firstListElementsToCommaDelimitedString(failedShards, s -> s.getRoutingEntry().shardId().toString(), logger.isDebugEnabled()); + return buildResultAndLogHealthChange(clusterState, allocation, "shards failed [" + failedShardsAsString + "]"); } /** @@ -317,13 +319,14 @@ public class AllocationService { * @param The list element type. * @return A comma-separated string of the first few elements. */ - private String firstListElementsToCommaDelimitedString(List elements, Function formatter) { + static String firstListElementsToCommaDelimitedString(List elements, Function formatter, boolean isDebugEnabled) { final int maxNumberOfElements = 10; - return elements - .stream() - .limit(maxNumberOfElements) - .map(formatter) - .collect(Collectors.joining(", ")); + if (isDebugEnabled || elements.size() <= maxNumberOfElements) { + return elements.stream().map(formatter).collect(Collectors.joining(", ")); + } else { + return elements.stream().limit(maxNumberOfElements).map(formatter).collect(Collectors.joining(", ")) + + ", ... [" + elements.size() + " items in total]"; + } } public CommandsResult reroute(final ClusterState clusterState, AllocationCommands commands, boolean explain, boolean retryFailed) { diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationServiceTests.java new file mode 100644 index 00000000000..6f6b6bb39bd --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AllocationServiceTests.java @@ -0,0 +1,72 @@ +/* + * 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.routing.allocation; + +import org.elasticsearch.test.ESTestCase; + +import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class AllocationServiceTests extends ESTestCase { + + public void testFirstListElementsToCommaDelimitedStringReportsAllElementsIfShort() { + List strings = IntStream.range(0, between(0, 10)).mapToObj(i -> randomAlphaOfLength(10)).collect(Collectors.toList()); + assertAllElementsReported(strings, randomBoolean()); + } + + public void testFirstListElementsToCommaDelimitedStringReportsAllElementsIfDebugEnabled() { + List strings = IntStream.range(0, between(0, 100)).mapToObj(i -> randomAlphaOfLength(10)).collect(Collectors.toList()); + assertAllElementsReported(strings, true); + } + + private void assertAllElementsReported(List strings, boolean isDebugEnabled) { + final String abbreviated = AllocationService.firstListElementsToCommaDelimitedString(strings, Function.identity(), isDebugEnabled); + for (String string : strings) { + assertThat(abbreviated, containsString(string)); + } + assertThat(abbreviated, not(containsString("..."))); + } + + public void testFirstListElementsToCommaDelimitedStringReportsFirstElementsIfLong() { + List strings = IntStream.range(0, between(11, 100)).mapToObj(i -> randomAlphaOfLength(10)) + .distinct().collect(Collectors.toList()); + final String abbreviated = AllocationService.firstListElementsToCommaDelimitedString(strings, Function.identity(), false); + for (int i = 0; i < strings.size(); i++) { + if (i < 10) { + assertThat(abbreviated, containsString(strings.get(i))); + } else { + assertThat(abbreviated, not(containsString(strings.get(i)))); + } + } + assertThat(abbreviated, containsString("...")); + assertThat(abbreviated, containsString("[" + strings.size() + " items in total]")); + } + + public void testFirstListElementsToCommaDelimitedStringUsesFormatterNotToString() { + List strings = IntStream.range(0, between(1, 100)).mapToObj(i -> "original").collect(Collectors.toList()); + final String abbreviated = AllocationService.firstListElementsToCommaDelimitedString(strings, s -> "formatted", randomBoolean()); + assertThat(abbreviated, containsString("formatted")); + assertThat(abbreviated, not(containsString("original"))); + } +}