[
https://issues.apache.org/jira/browse/HADOOP-9691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13699522#comment-13699522
]
Chris Nauroth commented on HADOOP-9691:
---------------------------------------
{quote}
Instead of returning null Integer, we could return -1, given callId always
starts with 0. You could also say a negative callId means invalid number in
the javadoc.
{quote}
This would be nice and consistent with what you're doing in HADOOP-9688 to
return a non-null dummy client ID. Unfortunately, there is no bounds checking
on the call ID, so a long-lived client will eventually overflow and start
legitimately sending negative numbers. Additionally, -1 is already reserved to
identify pings. I'm guessing this also means we have a long-standing bug where
a client can overflow and send -1, and the server will misinterpret it as a
ping. (It's probably a very rare bug, because a client would have to generate
2^32 RPC calls to hit it.)
How about this? We could reserve -2 to mean invalid/no active RPC call. Then,
I could add logic in the client after the getAndIncrement to check if it got -1
or -2, and if so, retry the getAndIncrement to get a valid call ID. There is
still a risk that an old client could send -2, but considering that we'll be
upgrading clients anyway to get the new client ID, the impact is small.
If you agree with this approach, then I'd like to hold off on this patch, wait
for your HADOOP-9688 patch to get committed, and then rebase. (Your new
{{RpcConstants}} class is the logical place to define {{INVALID_CALL_ID}}.)
> RPC clients can generate call ID using AtomicInteger instead of synchronizing
> on the Client instance.
> -----------------------------------------------------------------------------------------------------
>
> Key: HADOOP-9691
> URL: https://issues.apache.org/jira/browse/HADOOP-9691
> Project: Hadoop Common
> Issue Type: Improvement
> Components: ipc
> Affects Versions: 3.0.0, 2.1.0-beta
> Reporter: Chris Nauroth
> Assignee: Chris Nauroth
> Priority: Minor
> Attachments: HADOOP-9691.1.patch
>
>
> As noted in discussion on HADOOP-9688, we can optimize generation of call ID
> in the RPC client code. Currently, it synchronizes on the {{Client}}
> instance to coordinate access to a shared {{int}}. We can switch this to
> {{AtomicInteger}} to avoid lock contention.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira