Skip to content

Add information when master node left to DiscoveryNodes' shortSummary() #28197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 22, 2018
Merged

Add information when master node left to DiscoveryNodes' shortSummary() #28197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fddae48
Add information when master node left to DiscoveryNodes' shortSummary()
tlrx Jan 12, 2018
bf9e1b1
Apply feedback
tlrx Jan 15, 2018
5c814b7
Apply feedback 3
tlrx Jan 18, 2018
6d2faea
print empty brackets instead of [null]
tlrx Jan 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 42 additions & 50 deletions server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/**
* This class holds all {@link DiscoveryNode} in the cluster and provides convenience methods to
Expand Down Expand Up @@ -205,12 +206,14 @@ public DiscoveryNode getLocalNode() {
}

/**
* Get the master node
*
* @return master node
* Returns the master node, or {@code null} if there is no master node
*/
@Nullable
public DiscoveryNode getMasterNode() {
return nodes.get(masterNodeId);
if (masterNodeId != null) {
return nodes.get(masterNodeId);
}
return null;
}

/**
Expand Down Expand Up @@ -385,27 +388,20 @@ public DiscoveryNodes newNode(DiscoveryNode node) {
* Returns the changes comparing this nodes to the provided nodes.
*/
public Delta delta(DiscoveryNodes other) {
List<DiscoveryNode> removed = new ArrayList<>();
List<DiscoveryNode> added = new ArrayList<>();
final List<DiscoveryNode> removed = new ArrayList<>();
final List<DiscoveryNode> added = new ArrayList<>();
for (DiscoveryNode node : other) {
if (!this.nodeExists(node)) {
if (this.nodeExists(node) == false) {
removed.add(node);
}
}
for (DiscoveryNode node : this) {
if (!other.nodeExists(node)) {
if (other.nodeExists(node) == false) {
added.add(node);
}
}
DiscoveryNode previousMasterNode = null;
DiscoveryNode newMasterNode = null;
if (masterNodeId != null) {
if (other.masterNodeId == null || !other.masterNodeId.equals(masterNodeId)) {
previousMasterNode = other.getMasterNode();
newMasterNode = getMasterNode();
}
}
return new Delta(previousMasterNode, newMasterNode, localNodeId, Collections.unmodifiableList(removed),

return new Delta(other.getMasterNode(), getMasterNode(), localNodeId, Collections.unmodifiableList(removed),
Collections.unmodifiableList(added));
}

Expand All @@ -429,8 +425,8 @@ public String toString() {
public static class Delta {

private final String localNodeId;
private final DiscoveryNode previousMasterNode;
private final DiscoveryNode newMasterNode;
@Nullable private final DiscoveryNode previousMasterNode;
@Nullable private final DiscoveryNode newMasterNode;
private final List<DiscoveryNode> removed;
private final List<DiscoveryNode> added;

Expand All @@ -448,13 +444,15 @@ public boolean hasChanges() {
}

public boolean masterNodeChanged() {
return newMasterNode != null;
return Objects.equals(newMasterNode, previousMasterNode) == false;
}

@Nullable
public DiscoveryNode previousMasterNode() {
return previousMasterNode;
}

@Nullable
public DiscoveryNode newMasterNode() {
return newMasterNode;
}
Expand All @@ -476,51 +474,45 @@ public List<DiscoveryNode> addedNodes() {
}

public String shortSummary() {
StringBuilder sb = new StringBuilder();
if (!removed() && masterNodeChanged()) {
if (newMasterNode.getId().equals(localNodeId)) {
// we are the master, no nodes we removed, we are actually the first master
sb.append("new_master ").append(newMasterNode());
} else {
// we are not the master, so we just got this event. No nodes were removed, so its not a *new* master
sb.append("detected_master ").append(newMasterNode());
final StringBuilder summary = new StringBuilder();
if (masterNodeChanged()) {
summary.append("master node changed {previous [");
if (previousMasterNode() != null) {
summary.append(previousMasterNode());
}
} else {
if (masterNodeChanged()) {
sb.append("master {new ").append(newMasterNode());
if (previousMasterNode() != null) {
sb.append(", previous ").append(previousMasterNode());
}
sb.append("}");
summary.append("], current [");
if (newMasterNode() != null) {
summary.append(newMasterNode());
}
if (removed()) {
if (masterNodeChanged()) {
sb.append(", ");
}
sb.append("removed {");
for (DiscoveryNode node : removedNodes()) {
sb.append(node).append(',');
}
sb.append("}");
summary.append("]}");
}
if (removed()) {
if (summary.length() > 0) {
summary.append(", ");
}
summary.append("removed {");
for (DiscoveryNode node : removedNodes()) {
summary.append(node).append(',');
}
summary.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))) {
if (removed() || masterNodeChanged()) {
sb.append(", ");
if (summary.length() > 0) {
summary.append(", ");
}
sb.append("added {");
summary.append("added {");
for (DiscoveryNode node : addedNodes()) {
if (!node.getId().equals(localNodeId)) {
// don't print ourself
sb.append(node).append(',');
summary.append(node).append(',');
}
}
sb.append("}");
summary.append("}");
}
}
return sb.toString();
return summary.toString();
}
}

Expand Down
7 changes: 3 additions & 4 deletions server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,15 +140,14 @@ public void testDeltas() {

DiscoveryNodes.Delta delta = discoNodesB.delta(discoNodesA);

if (masterB == null || Objects.equals(masterAId, masterBId)) {
if (Objects.equals(masterAId, masterBId)) {
assertFalse(delta.masterNodeChanged());
assertThat(delta.previousMasterNode(), nullValue());
assertThat(delta.newMasterNode(), nullValue());
} else {
assertTrue(delta.masterNodeChanged());
assertThat(delta.newMasterNode().getId(), equalTo(masterBId));
assertThat(delta.previousMasterNode() != null ? delta.previousMasterNode().getId() : null,
equalTo(masterAId));
assertThat(delta.newMasterNode() != null ? delta.newMasterNode().getId() : null, equalTo(masterBId));
assertThat(delta.previousMasterNode() != null ? delta.previousMasterNode().getId() : null, equalTo(masterAId));
}

Set<DiscoveryNode> newNodes = new HashSet<>(nodesB);
Expand Down