[
https://issues.apache.org/jira/browse/HADOOP-16700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16975457#comment-16975457
]
Erik Krogen edited comment on HADOOP-16700 at 11/15/19 10:58 PM:
-----------------------------------------------------------------
Thanks for the explanation [~xuzq_zander]! Very helpful. It definitely seems
like a valid issue.
I took a look at the v001 patch. By the way, please follow the [patch naming
conventions|https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute#HowToContribute-Namingyourpatch]
-- it should be {{HADOOP-16700.001.patch}} (period instead of hyphen before
the version, and you don't need to specify a branch when it is trunk).
The general approach seems sound to me. I am concerned about all of the changes
you've made to the signatures of methods, removing {{receiveTime}}. First off
{{Server}} is a public interface, so we should not make breaking changes to its
API. To introduce a new method here, you need to keep the old one but mark it
as {{@Deprecated}}. Second off, this change seems unrelated to this JIRA? If
that is the case, we should keep it separate.
My only other comment is that we should update the comments here within
{{Call}}:
{code}
long timestampNanos; // time received when response is null
// time served when response is not null
long responseTimestampNanos;
{code}
I think that it would now be more accurate to say:
{code}
long timestampNanos; // time the call was received
long responseTimestampNanos; // time the call was served
{code}
Let me know if you think that isn't correct.
was (Author: xkrogen):
Thanks for the explanation [~xuzq_zander]! It definitely seems like a valid
issue.
I took a look at the v001 patch. By the way, please follow the [patch naming
conventions|https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute#HowToContribute-Namingyourpatch]
-- it should be {{HADOOP-16700.001.patch}} (period instead of hyphen before
the version, and you don't need to specify a branch when it is trunk).
The general approach seems sound to me. I am concerned about all of the changes
you've made to the signatures of methods, removing {{receiveTime}}. First off
{{Server}} is a public interface, so we should not make breaking changes to its
API. To introduce a new method here, you need to keep the old one but mark it
as {{@Deprecated}}. Second off, this change seems unrelated to this JIRA? If
that is the case, we should keep it separate.
My only other comment is that we should update the comments here within
{{Call}}:
{code}
long timestampNanos; // time received when response is null
// time served when response is not null
long responseTimestampNanos;
{code}
I think that it would now be more accurate to say:
{code}
long timestampNanos; // time the call was received
long responseTimestampNanos; // time the call was served
{code}
Let me know if you think that isn't correct.
> RpcQueueTime may be negative when the response has to be sent later
> -------------------------------------------------------------------
>
> Key: HADOOP-16700
> URL: https://issues.apache.org/jira/browse/HADOOP-16700
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: xuzq
> Assignee: xuzq
> Priority: Minor
> Attachments: HADOOP-16700-trunk-001.patch
>
>
> RpcQueueTime may be negative when the response has to be sent later.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
