[
https://issues.apache.org/jira/browse/ZOOKEEPER-2695?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17368712#comment-17368712
]
Damien Diederen commented on ZOOKEEPER-2695:
--------------------------------------------
Hi [~arshad.mohammad],
I was looking into this, following [this
question|https://github.com/apache/zookeeper/pull/1716#pullrequestreview-687896698]
of [~eolivelli]'s:
bq. You are also introducing a new error code, how do old clients will handle
that error code?
This is a recurrent question—one to which I feel we don't have a very good
answer.
The patch attached to this ticket still applies, provided one updates the paths
and the target JUnit API imports. \(I am attaching a refreshed version as I
have it at hand; feel free to turn it into a GitHub pull request.)
The proposed solution definitely improves on the status quo, but I have a few
questions:
# {{Code.SYSTEMERROR}} is documented as follows:
bq. This is never thrown by the server, it shouldn't be used other than to
indicate a range. Specifically error codes greater than this value, but lesser
than {{#APIERROR}}, are system errors.
So I suppose I would suggest throwing {{SystemErrorException}} for codes in
{{\[Code.SYSTEMERROR, Code.APIERROR)}} and {{APIErrorException}} codes outside
that range. What do you think?
# Unfortunately, "known" system errors such as
{{RuntimeInconsistencyException}} do not inherit from {{SystemErrorException}}.
Similarly, an "API error" such as {{NoNodeException}} does not inherit from
{{APIErrorException}}.
The list of {{KeeperException}} subclasses being flat means that clients have
intimate knowledge of error codes to distinguish between exceptions which
warrant an immediate retry, exceptions which should trigger exponential
back-off, or fatal ones such as {{Code.AUTHFAILED}}.
While reparenting the existing classes would technically be an API/ABI break,
we could potentially introduce superclasses such as e.g.
{{BaseSystemErrorException}} between {{KeeperException}} and its children
\(AFAICT, Sun & Oracle frequently do so), and use ranges to map unknown codes.
Is that something we should consider?
# {{KeeperException}} currently uses a {{private Code code}} member variable,
which makes it impossible to propagate an unknown error codes.
Should we change that \(back?) to an {{int}}—without changing the public
interface?
Of course, methods such as {{public Code code()}} would still have to return
{{null}}, but others such as {{public int getCode()}} \(currently deprecated)
could return the actual value, and an informative message could still be
generated in {{public String getMessage()}}.
Thoughts?
> Handle unknown error for rolling upgrade old client new server scenario
> -----------------------------------------------------------------------
>
> Key: ZOOKEEPER-2695
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2695
> Project: ZooKeeper
> Issue Type: Bug
> Components: java client
> Reporter: Mohammad Arshad
> Assignee: Mohammad Arshad
> Priority: Major
> Attachments:
> 0001-ZOOKEEPER-2695-Handle-unknown-error-for-rolling-upgr.patch,
> ZOOKEEPER-2695-01.patch
>
>
> In Zookeeper rolling upgrade scenario where server is new but client is old,
> when sever sends error code which is not understood by the client, client
> throws NullPointerException.
> KeeperException.SystemErrorException should be thrown for all unknown error
> code.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
