[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2627?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15641224#comment-15641224
 ] 

Hadoop QA commented on ZOOKEEPER-2627:
--------------------------------------

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12837642/ZOOKEEPER-2627.patch
  against trunk revision bcb07a09b06c91243ed244f04a71b8daf629e286.

    +1 @author.  The patch does not contain any @author tags.

    -1 tests included.  The patch doesn't appear to include any new or modified 
tests.
                        Please justify why no new tests are needed for this 
patch.
                        Also please list what manual steps were performed to 
verify this patch.

    +1 javadoc.  The javadoc tool did not generate any warning messages.

    +1 javac.  The applied patch does not increase the total number of javac 
compiler warnings.

    -1 findbugs.  The patch appears to introduce 19 new Findbugs (version 
3.0.1) warnings.

    +1 release audit.  The applied patch does not increase the total number of 
release audit warnings.

    +1 core tests.  The patch passed core unit tests.

    +1 contrib tests.  The patch passed contrib unit tests.

Test results: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3523//testReport/
Findbugs warnings: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3523//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: 
https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3523//console

This message is automatically generated.

> Remove ZRWSERVERFOUND from C client and replace handle_error with something 
> more semantically explicit for r/w server reconnect.
> --------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: ZOOKEEPER-2627
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2627
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: c client
>    Affects Versions: 3.5.2
>            Reporter: Michael Han
>            Assignee: Michael Han
>             Fix For: 3.5.3
>
>         Attachments: ZOOKEEPER-2627.patch, ZOOKEEPER-2627.patch
>
>
> While working on ZOOKEEPER-2014, I noticed a discrepancy between Java and C 
> client regarding the error codes definition. There is a 
> {noformat}ZRWSERVERFOUND = -122{noformat} definition in C client which is not 
> present in Java client's KeeperException.Code definitions. 
> This discrepancy was introduced by ZOOKEEPER-827, where the C client logic 
> was simulating the Java client's logic when doing a read/write server search 
> while client is in read only mode. Once client finds a valid read/write 
> server, client will try to disconnect and reconnect with this read/write 
> server, as we always prefer r/w server in ro mode. The way Java client is 
> doing this disconnect/reconnect process is by throwing a 
> RWServerFoundException (instead of a KeeperException) to set the client in 
> disconnected state, then wait for client reconnect with r/w server address 
> set before throwing the exception. C client did similar but instead of having 
> an explicitly disconnect / clean up routine, the client was relying on 
> handle_error to do the job where ZRWSERVERFOUND was introduced.
> I propose we remove ZRWSERVERFOUND error code from C client and use an 
> explicit routine instead of handle_error when we do r/w server search in C 
> client for two reasons:
> * ZRWSERVERFOUND is not something ZK client users would need to know. It's a 
> pure implementation detail that's used to alter the connection state of the 
> client, and ZK client users have no desire nor need to handle such errors, as 
> R/W server scanning and connect is handled transparently by ZK client library.
> * To maintain consistency between Java and C client regarding error codes 
> definition. Without removing this from C client, we would need to replace 
> RWServerFoundException in Java client with a new KeeperException, and again 
> with the reason mentioned above, we don't need a KeeperException for this 
> because such implementation detail does not have to be exposed to end users 
> (unless, we provided alternative for users to opt-out automate R/W server 
> switching when in read only mode which we don't.).



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to