shayshim commented on a change in pull request #345: [CURATOR-551] Fix Handle
Holder new connection string
URL: https://github.com/apache/curator/pull/345#discussion_r378533404
##########
File path: curator-client/src/main/java/org/apache/curator/HandleHolder.java
##########
@@ -70,7 +61,16 @@ String getConnectionString()
String getNewConnectionString()
{
String helperConnectionString = (helper != null) ?
helper.getConnectionString() : null;
- return ((helperConnectionString != null) &&
!ensembleProvider.getConnectionString().equals(helperConnectionString)) ?
helperConnectionString : null;
+ String ensembleProviderConnectionString =
ensembleProvider.getConnectionString();
Review comment:
> `closeAndReset()` will cause `ensembleProvider.getConnectionString()` to
get called anyway. Does that answer your question? I think the intent of the
above code is to force-close the connection when there's a new connection
string.
In short I wanted to make sure that `closeAndReset()` (create `helper`) is
called before any new connection string is set .
As @cammckenzie said, it is called from `ConnectionState.start()`, which is
called (eventually) from `CuratorFrameworkImpl.start()`, from line 338 using
`client.start()` - before `ensembleTracker.start()` in line 353, which allows
to set new connection strings.
So any call to `getNewConnectionString()`, with actual new connection
string, will meet non `null` `helper`.
In addition to that, `helper.getZooKeeper()` is also called eventually from
338, so `data.connectionString` is set as needed, before
`getNewConnectionString()` can be called.
So I think we have no issue here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
