[ 
https://issues.apache.org/jira/browse/CURATOR-551?focusedWorklogId=386256&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-386256
 ]

ASF GitHub Bot logged work on CURATOR-551:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 12/Feb/20 21:51
            Start Date: 12/Feb/20 21:51
    Worklog Time Spent: 10m 
      Work Description: shayshim commented on 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()` 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]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 386256)
    Time Spent: 2.5h  (was: 2h 20m)

> Curator client always call updateServerList with original serverList value, 
> not the ones updated by EnsembleTracker
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: CURATOR-551
>                 URL: https://issues.apache.org/jira/browse/CURATOR-551
>             Project: Apache Curator
>          Issue Type: Bug
>          Components: Client
>    Affects Versions: 4.2.0
>            Reporter: kelgon wu
>            Assignee: Jordan Zimmerman
>            Priority: Major
>          Time Spent: 2.5h
>  Remaining Estimate: 0h
>
> Whenever ConnectionState.handleNewConnectionString is triggered, the value of 
>  newConnectionString is always the value used to construct the 
> FixedEnsembleProvider
>  
> in ConnectionState:
> {code:java}
> String newConnectionString = zooKeeper.getNewConnectionString();
> if ( newConnectionString != null ) {
>     handleNewConnectionString(newConnectionString);
> }
> ...
> if ( ensembleProvider.updateServerListEnabled() ) {
>     zooKeeper.updateServerList(newConnectionString);
> } else {
>     reset();
> }{code}
>  
> in HandleHolder:
> {code:java}
> String getNewConnectionString() {
>     String helperConnectionString = (helper != null) ? 
> helper.getConnectionString() : null;
>     return ((helperConnectionString != null) && 
> !ensembleProvider.getConnectionString().equals(helperConnectionString)) ? 
> helperConnectionString : null;
> }{code}
> code above shows that the newConnectionString is provided by 
> helper.getConnectionString(), which only instantiated on first call of 
> getZookeeper:
> {code:java}
> @Override
> public ZooKeeper getZooKeeper() throws Exception
> {
>     synchronized(this)
>     {
>         if ( zooKeeperHandle == null )
>         {
>             connectionString = ensembleProvider.getConnectionString();
>             zooKeeperHandle = zookeeperFactory.newZooKeeper(connectionString, 
> sessionTimeout, watcher, canBeReadOnly);
>         }
> ...{code}
>  
> As a result, when I reconfig zookeeper serverList, curator client always call 
> updateServerList with the initial value, not the ones I just reconfigured



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to