[jira] [Commented] (ZOOKEEPER-2627) Remove ZRWSERVERFOUND from C client and replace handle_error with something more semantically explicit for r/w server reconnect.

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

ASF GitHub Bot commented on ZOOKEEPER-2627:
-------------------------------------------

GitHub user hanm opened a pull request:

    https://github.com/apache/zookeeper/pull/100

    ZOOKEEPER-2627:Remove ZRWSERVERFOUND from C client.

    JIRA: https://issues.apache.org/jira/browse/ZOOKEEPER-2627
    
    * Remove ZRWSERVERFOUND from C client to maintain consistency of error codes definition between Java / C client.
    * Make C client behavior more conforming with Java client in RO mode by having an explicit RW server address and use that address whenever it's available.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hanm/zookeeper ZOOKEEPER-2627

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/100.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #100
    
----
commit 81a0de2298efdce83061faf6f6a4b0b89e245b64
Author: Michael Han <ha...@cloudera.com>
Date:   2016-11-04T22:34:55Z

    ZOOKEEPER-2627:Remove ZRWSERVERFOUND from C client.

----


> 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
>
>
> 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)