Skip to content

Commit 42a52a6

Browse files
symatsymat
committed
ZOOKEEPER-3188: improve based on code review comments
1 parent 6c4220a commit 42a52a6

5 files changed

Lines changed: 80 additions & 40 deletions

File tree

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java‎

Lines changed: 44 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,19 @@
1919
package org.apache.zookeeper.server.admin;
2020

2121
import java.net.InetSocketAddress;
22-
import java.util.*;
22+
import java.util.Arrays;
23+
import java.util.Collections;
24+
import java.util.HashMap;
25+
import java.util.HashSet;
26+
import java.util.List;
27+
import java.util.Map;
28+
import java.util.Properties;
29+
import java.util.Set;
30+
import java.util.SortedMap;
31+
import java.util.TreeMap;
2332
import java.util.stream.Collectors;
2433

25-
import com.fasterxml.jackson.annotation.JsonAnyGetter;
34+
import com.fasterxml.jackson.annotation.JsonProperty;
2635
import org.apache.zookeeper.Environment;
2736
import org.apache.zookeeper.Environment.Entry;
2837
import org.apache.zookeeper.Version;
@@ -36,7 +45,9 @@
3645
import org.apache.zookeeper.server.quorum.FollowerZooKeeperServer;
3746
import org.apache.zookeeper.server.quorum.Leader;
3847
import org.apache.zookeeper.server.quorum.LeaderZooKeeperServer;
48+
import org.apache.zookeeper.server.quorum.MultipleAddresses;
3949
import org.apache.zookeeper.server.quorum.QuorumPeer;
50+
import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
4051
import org.apache.zookeeper.server.quorum.QuorumZooKeeperServer;
4152
import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;
4253
import org.apache.zookeeper.server.quorum.ReadOnlyZooKeeperServer;
@@ -620,57 +631,53 @@ public CommandResponse run(ZooKeeperServer zkServer, Map<String, String> kwargs)
620631
CommandResponse response = initializeResponse();
621632
if (zkServer instanceof QuorumZooKeeperServer) {
622633
QuorumPeer peer = ((QuorumZooKeeperServer) zkServer).self;
623-
VotingView votingView = new VotingView(peer.getVotingView());
634+
Map<Long, QuorumServerView> votingView = peer.getVotingView().entrySet().stream()
635+
.collect(Collectors.toMap(Map.Entry::getKey, e -> new QuorumServerView(e.getValue())));
624636
response.put("current_config", votingView);
625637
} else {
626638
response.put("current_config", Collections.emptyMap());
627639
}
628640
return response;
629641
}
630642

643+
private static class QuorumServerView {
631644

632-
private static class VotingView {
633-
private final Map<Long, String> view;
645+
@JsonProperty
646+
private List<String> serverAddresses;
634647

635-
VotingView(Map<Long,QuorumPeer.QuorumServer> view) {
636-
this.view = view.entrySet().stream()
637-
.filter(e -> e.getValue().addr != null)
638-
.collect(Collectors.toMap(Map.Entry::getKey,
639-
e -> String.format("%s:%s%s",
640-
getMultiAddressString(e.getValue()),
641-
e.getValue().type.equals(QuorumPeer.LearnerType.PARTICIPANT) ? "participant" : "observer",
642-
e.getValue().clientAddr ==null || e.getValue().isClientAddrFromStatic ? "" :
643-
String.format(";%s:%d",
644-
QuorumPeer.QuorumServer.delimitedHostString(e.getValue().clientAddr),
645-
e.getValue().clientAddr.getPort())),
646-
(v1, v2) -> v1, // cannot get duplicates as this straight draws from the other map
647-
TreeMap::new));
648-
}
649-
650-
private String getMultiAddressString(QuorumPeer.QuorumServer qs) {
651-
return qs.addr.getAllAddresses().stream()
652-
.map(address -> getSingleAddressString(qs, address))
653-
.collect(Collectors.joining(","));
654-
}
648+
@JsonProperty
649+
private List<String> electionAddresses;
655650

656-
private String getSingleAddressString(QuorumPeer.QuorumServer qs, InetSocketAddress address) {
657-
final String addressHostString = address.getHostString();
658-
final String delimitedHostString = QuorumPeer.QuorumServer.delimitedHostString(address);
651+
@JsonProperty
652+
private String clientAddress;
659653

660-
Optional<InetSocketAddress> matchingElectionAddress = qs.electionAddr.getAllAddresses().stream()
661-
.filter(electionAddress -> electionAddress.getHostString().equals(addressHostString))
662-
.findFirst();
663-
final String electionPort = matchingElectionAddress.map(e-> ":" + e.getPort()).orElse("");
654+
@JsonProperty
655+
private String learnerType;
664656

665-
return String.format("%s:%d%s", delimitedHostString, address.getPort(), electionPort);
657+
public QuorumServerView(QuorumPeer.QuorumServer quorumServer) {
658+
this.serverAddresses = getMultiAddressString(quorumServer.addr);
659+
this.electionAddresses = getMultiAddressString(quorumServer.electionAddr);
660+
this.learnerType = quorumServer.type.equals(LearnerType.PARTICIPANT) ? "participant" : "observer";
661+
this.clientAddress = getAddressString(quorumServer.clientAddr);
666662
}
667663

668-
@JsonAnyGetter
669-
public Map<Long, String> getView() {
670-
return view;
664+
private static List<String> getMultiAddressString(MultipleAddresses multipleAddresses) {
665+
if(multipleAddresses == null) {
666+
return Collections.emptyList();
667+
}
668+
669+
return multipleAddresses.getAllAddresses().stream()
670+
.map(QuorumServerView::getAddressString)
671+
.collect(Collectors.toList());
671672
}
672-
}
673673

674+
private static String getAddressString(InetSocketAddress address) {
675+
if(address == null) {
676+
return "";
677+
}
678+
return String.format("%s:%d", QuorumPeer.QuorumServer.delimitedHostString(address), address.getPort());
679+
}
680+
}
674681

675682
}
676683

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ ServerSocket createServerSocket(InetSocketAddress address, boolean portUnificati
311311
serverSocket.bind(address);
312312
return serverSocket;
313313
} catch (BindException e) {
314-
LOG.error("Couldn't bind to " + self.getQuorumAddress(), e);
314+
LOG.error("Couldn't bind to " + address.toString(), e);
315315
throw e;
316316
}
317317
}

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ public class QuorumCnxManager {
114114
/*
115115
* Protocol identifier used among peers
116116
*/
117-
public static final long PROTOCOL_VERSION = -65536L;
117+
public static final long PROTOCOL_VERSION = -65535L;
118118

119119
/*
120120
* Max buffer size to be read from the network.

‎zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,12 @@ public void testWatchSummary() throws IOException, InterruptedException {
292292
new Field("num_total_watches", Integer.class));
293293
}
294294

295+
@Test
296+
public void testVotingViewCommand() throws IOException, InterruptedException {
297+
testCommand("voting_view",
298+
new Field("current_config", Map.class));
299+
}
300+
295301
@Test
296302
public void testConsCommandSecureOnly() {
297303
// Arrange

‎zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.nio.ByteBuffer;
3030
import java.nio.channels.SocketChannel;
3131
import java.util.ArrayList;
32+
import java.util.Arrays;
3233
import java.util.Date;
3334
import java.util.HashMap;
3435
import java.util.Map;
@@ -58,6 +59,8 @@
5859
import org.junit.Before;
5960
import org.junit.Test;
6061

62+
import static org.junit.Assert.assertEquals;
63+
6164
public class CnxManagerTest extends ZKTestCase {
6265
protected static final Logger LOG = LoggerFactory.getLogger(FLENewEpochTest.class);
6366
protected static final int THRESHOLD = 4;
@@ -608,7 +611,7 @@ public void testInitialMessage() throws Exception {
608611
Assert.fail("bad hostport accepted");
609612
} catch (InitialMessage.InitialMessageException ex) {}
610613

611-
// good message
614+
// good message, single election address
612615
try {
613616

614617
hostport = "10.0.0.2:3888";
@@ -621,6 +624,30 @@ public void testInitialMessage() throws Exception {
621624
// now parse it
622625
din = new DataInputStream(new ByteArrayInputStream(bos.toByteArray()));
623626
msg = InitialMessage.parse(QuorumCnxManager.PROTOCOL_VERSION, din);
627+
assertEquals(new Long(5L), msg.sid);
628+
assertEquals(Arrays.asList(new InetSocketAddress("10.0.0.2", 3888)), msg.electionAddr);
629+
} catch (InitialMessage.InitialMessageException ex) {
630+
Assert.fail(ex.toString());
631+
}
632+
633+
// good message, multiple election addresses (ZOOKEEPER-3188)
634+
try {
635+
636+
hostport = "1.1.1.1:9999,2.2.2.2:8888,3.3.3.3:7777";
637+
bos = new ByteArrayOutputStream();
638+
dout = new DataOutputStream(bos);
639+
dout.writeLong(5L); // sid
640+
dout.writeInt(hostport.getBytes().length);
641+
dout.writeBytes(hostport);
642+
643+
// now parse it
644+
din = new DataInputStream(new ByteArrayInputStream(bos.toByteArray()));
645+
msg = InitialMessage.parse(QuorumCnxManager.PROTOCOL_VERSION, din);
646+
assertEquals(new Long(5L), msg.sid);
647+
assertEquals(Arrays.asList(new InetSocketAddress("1.1.1.1", 9999),
648+
new InetSocketAddress("2.2.2.2", 8888),
649+
new InetSocketAddress("3.3.3.3", 7777)),
650+
msg.electionAddr);
624651
} catch (InitialMessage.InitialMessageException ex) {
625652
Assert.fail(ex.toString());
626653
}

0 commit comments

Comments
 (0)