[
https://issues.apache.org/jira/browse/ZOOKEEPER-1485?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15328679#comment-15328679
]
Martin Kuchta commented on ZOOKEEPER-1485:
------------------------------------------
I was thinking about the value of a test case around the overflow condition
here. Like you said, we would need to add a set_xid function and move the xid
variable outside the get_xid function to let both functions access it. I didn't
see an existing pattern to add test-only code either. Do you have any
suggestions?
As for the performance issue, that's also a valid point. I was a bit hesitant
to replace the atomic operation at first, but I'm not sure it's actually an
issue. The locking happens once per client request, the lock is held for a very
short period of time, and each request already performs other locking on the ZK
handle (enter_critical(), lock_buffer_list()). The one difference here might be
that this is a global lock not tied to a particular zhandle, which could cause
performance issues with multiple threads making requests with different
zhandles.
I don't think you can correctly implement this using a plain CAS and atomic
add. Any way you combine those operations, I think there's a chance the CAS
won't trigger on the value you're checking against. Did you have a specific
implementation in mind? I might be approaching it from the wrong angle when
trying to reason about possible implementations.
> client xid overflow is not handled
> ----------------------------------
>
> Key: ZOOKEEPER-1485
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1485
> Project: ZooKeeper
> Issue Type: Bug
> Components: c client, java client
> Affects Versions: 3.4.3, 3.3.5
> Reporter: Michi Mutsuzaki
> Assignee: Martin Kuchta
> Attachments: ZOOKEEPER-1485.patch
>
>
> Both Java and C clients use signed 32-bit int as XIDs. XIDs are assumed to be
> non-negative, and zookeeper uses some negative values as special XIDs (e.g.
> -2 for ping, -4 for auth). However, neither Java nor C client ensures the
> XIDs it generates are non-negative, and the server doesn't reject negative
> XIDs.
> Pat had some suggestions on how to fix this:
> - (bin-compat) Expire the session when the client sends a negative XID.
> - (bin-incompat) In addition to expiring the session, use 64-bit int for XID
> so that overflow will practically never happen.
> --Michi
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
