honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove trailing comma from nodes lists (#46484) 

Today when the membership of the cluster changes we log messages that describe
the change like this:

    added {{node-1}{OPdaTIGmSxaEXXOyg3o96w}{127.0.0.1}{127.0.0.1:9301}{di},}

The trailing comma suggests there is some missing string that might contain
extra information, but in fact it's an artefact of how these messages are
constructed. This commit removes the trailing comma from these lists.
This commit is contained in:
David Turner 2019-09-09 14:46:49 +01:00
parent ee3396735c
commit 8428f8e6e8
4 changed files with 44 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
server/src/main/java/org/elasticsearch/cluster/coordination/NodeRemovalClusterStateTaskExecutor.java
View File

@ -56,7 +56,7 @@ public class NodeRemovalClusterStateTaskExecutor implements ClusterStateTaskExec
@Override
public String toString() {
return node + " " + reason;
return node + " reason: " + reason;
}
}

24
server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java
View File

@ -27,6 +27,7 @@ import org.elasticsearch.cluster.AbstractDiffable;
import org.elasticsearch.cluster.Diff;
import org.elasticsearch.common.Booleans;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
@ -41,6 +42,7 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
@ -517,26 +519,18 @@ public class DiscoveryNodes extends AbstractDiffable<DiscoveryNodes> implements
if (summary.length() > 0) {
summary.append(", ");
}
summary.append("removed {");
for (DiscoveryNode node : removedNodes()) {
summary.append(node).append(',');
}
summary.append("}");
summary.append("removed {").append(Strings.collectionToCommaDelimitedString(removedNodes())).append('}');
}
if (added()) {
// don't print if there is one added, and it is us
if (!(addedNodes().size() == 1 && addedNodes().get(0).getId().equals(localNodeId))) {
final String addedNodesExceptLocalNode = addedNodes().stream()
.filter(node -> node.getId().equals(localNodeId) == false).map(DiscoveryNode::toString)
.collect(Collectors.joining(","));
if (addedNodesExceptLocalNode.length() > 0) {
// ignore ourselves when reporting on nodes being added
if (summary.length() > 0) {
summary.append(", ");
}
summary.append("added {");
for (DiscoveryNode node : addedNodes()) {
if (!node.getId().equals(localNodeId)) {
// don't print ourself
summary.append(node).append(',');
}
}
summary.append("}");
summary.append("added {").append(addedNodesExceptLocalNode).append('}');
}
}
return summary.toString();

8
server/src/main/java/org/elasticsearch/cluster/service/MasterService.java
View File

@ -239,10 +239,10 @@ public class MasterService extends AbstractLifecycleComponent {
// new cluster state, notify all listeners
final DiscoveryNodes.Delta nodesDelta = clusterChangedEvent.nodesDelta();
if (nodesDelta.hasChanges() && logger.isInfoEnabled()) {
String nodeSummary = nodesDelta.shortSummary();
if (nodeSummary.length() > 0) {
logger.info("{}, term: {}, version: {}, reason: {}",
summary, newClusterState.term(), newClusterState.version(), nodeSummary);
String nodesDeltaSummary = nodesDelta.shortSummary();
if (nodesDeltaSummary.length() > 0) {
logger.info("{}, term: {}, version: {}, delta: {}",
summary, newClusterState.term(), newClusterState.version(), nodesDeltaSummary);
}
}

30
server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java
View File

@ -44,6 +44,7 @@ import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.oneOf;
public class DiscoveryNodesTests extends ESTestCase {
@ -163,6 +164,35 @@ public class DiscoveryNodesTests extends ESTestCase {
assertEquals(sortedNodes, returnedNodes);
}
public void testDeltaListsMultipleNodes() {
final List<DiscoveryNode> discoveryNodes = randomNodes(3);
final DiscoveryNodes nodes0 = DiscoveryNodes.builder().add(discoveryNodes.get(0)).build();
final DiscoveryNodes nodes01 = DiscoveryNodes.builder(nodes0).add(discoveryNodes.get(1)).build();
final DiscoveryNodes nodes012 = DiscoveryNodes.builder(nodes01).add(discoveryNodes.get(2)).build();
assertThat(nodes01.delta(nodes0).shortSummary(), equalTo("added {" + discoveryNodes.get(1) + "}"));
assertThat(nodes012.delta(nodes0).shortSummary(), oneOf(
"added {" + discoveryNodes.get(1) + "," + discoveryNodes.get(2) + "}",
"added {" + discoveryNodes.get(2) + "," + discoveryNodes.get(1) + "}"));
assertThat(nodes0.delta(nodes01).shortSummary(), equalTo("removed {" + discoveryNodes.get(1) + "}"));
assertThat(nodes0.delta(nodes012).shortSummary(), oneOf(
"removed {" + discoveryNodes.get(1) + "," + discoveryNodes.get(2) + "}",
"removed {" + discoveryNodes.get(2) + "," + discoveryNodes.get(1) + "}"));
final DiscoveryNodes nodes01Local = DiscoveryNodes.builder(nodes01).localNodeId(discoveryNodes.get(1).getId()).build();
final DiscoveryNodes nodes02Local = DiscoveryNodes.builder(nodes012).localNodeId(discoveryNodes.get(1).getId()).build();
assertThat(nodes01Local.delta(nodes0).shortSummary(), equalTo(""));
assertThat(nodes02Local.delta(nodes0).shortSummary(), equalTo("added {" + discoveryNodes.get(2) + "}"));
assertThat(nodes0.delta(nodes01Local).shortSummary(), equalTo("removed {" + discoveryNodes.get(1) + "}"));
assertThat(nodes0.delta(nodes02Local).shortSummary(), oneOf(
"removed {" + discoveryNodes.get(1) + "," + discoveryNodes.get(2) + "}",
"removed {" + discoveryNodes.get(2) + "," + discoveryNodes.get(1) + "}"));
}
public void testDeltas() {
Set<DiscoveryNode> nodesA = new HashSet<>();
nodesA.addAll(randomNodes(1 + randomInt(10)));