zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Hadoop QA (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-2627) Remove ZRWSERVERFOUND from C client and replace handle_error with something more semantically explicit for r/w server reconnect.
Date Fri, 04 Nov 2016 23:44:58 GMT

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

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

-1 overall.  Here are the results of testing the latest attachment 
  http://issues.apache.org/jira/secure/attachment/12837305/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/3520//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3520//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/3520//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
>
>
> 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)

Mime
View raw message