tisonkun commented on code in PR #460:
URL: https://github.com/apache/curator/pull/460#discussion_r1199690493
##########
curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java:
##########
@@ -436,6 +439,51 @@ public void testNewMembers() throws Exception
}
}
+ @Test
+ public void testRemoveWithChroot() throws Exception
+ {
+ // Use a long chroot path to circumvent ZOOKEEPER-4565 and
ZOOKEEPER-4601
+ String chroot = "/pretty-long-chroot";
+
+ try (CuratorFramework client = newClient(cluster.getConnectString() +
chroot)) {
+ client.start();
+ client.create().forPath("/", "deadbeef".getBytes());
+
+ QuorumVerifier oldConfig =
toQuorumVerifier(client.getConfig().forEnsemble());
+ assertConfig(oldConfig, cluster.getInstances());
+
+ CountDownLatch latch = setChangeWaiter(client);
+
+ Collection<InstanceSpec> oldInstances = cluster.getInstances();
+ InstanceSpec us =
cluster.findConnectionInstance(client.getZookeeperClient().getZooKeeper());
+ InstanceSpec removeSpec = oldInstances.iterator().next();
+ if ( us.equals(removeSpec) ) {
+ Iterator<InstanceSpec> iterator = oldInstances.iterator();
+ iterator.next();
+ removeSpec = iterator.next();
+ }
+
+
client.reconfig().leaving(Integer.toString(removeSpec.getServerId())).fromConfig(oldConfig.getVersion()).forEnsemble();
+
+ assertTrue(timing.awaitLatch(latch));
+
+ byte[] newConfigData = client.getConfig().forEnsemble();
+ QuorumVerifier newConfig = toQuorumVerifier(newConfigData);
+ List<InstanceSpec> newInstances =
Lists.newArrayList(cluster.getInstances());
+ newInstances.remove(removeSpec);
+ assertConfig(newConfig, newInstances);
+
+ assertTrue(timing.awaitLatch(ensembleLatch));
+ String connectString =
EnsembleTracker.configToConnectionString(newConfig) + chroot;
+ assertEquals(connectString,
ensembleProvider.getConnectionString());
+
+ client.getZookeeperClient().reset();
+ client.sync().forPath("/");
+ byte[] data = client.getData().forPath("/");
+ assertArrayEquals("deadbeef".getBytes(), data, () -> "expected
\"deedbeef\", got data: " + Arrays.toString(data));
Review Comment:
```suggestion
assertThat(new String(data)).isEqualTo("deadbeef");
```
Simplify the assertion
##########
curator-framework/src/main/java/org/apache/curator/framework/imps/EnsembleTracker.java:
##########
@@ -209,10 +209,17 @@ private void processConfigData(byte[] data) throws
Exception
if (!properties.isEmpty())
{
QuorumMaj newConfig = new QuorumMaj(properties);
- String connectionString = configToConnectionString(newConfig);
- if (connectionString.trim().length() > 0)
+ String connectionString =
configToConnectionString(newConfig).trim();
+ if (!connectionString.isEmpty())
{
currentConfig.set(newConfig);
+ String oldConnectionString =
ensembleProvider.getConnectionString();
+ int i = oldConnectionString.indexOf('/');
+ if (i >= 0)
Review Comment:
Perhap `== 0`? I'm not sure what if `i > 0` the string constructing.
##########
curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java:
##########
@@ -558,6 +606,7 @@ private CuratorFramework newClient(String connectionString)
{
private CuratorFramework newClient(String connectionString, boolean
withEnsembleProvider)
{
final AtomicReference<String> connectString = new
AtomicReference<>(connectionString);
+ ensembleLatch = new CountDownLatch(1);
Review Comment:
I suggest to add a `newClient(String connectionString, boolean
withEnsembleProvider, CountDownLatch)` overload to avoid using field - this is
somehow global variable that is brittle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
