[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16498592#comment-16498592
]
Konstantin Shvachko edited comment on HDFS-13399 at 6/1/18 9:37 PM:
Sorry this was the previous build, which Plamen fixed.
Please disregard this comment. Builds are confusing.
was (Author: shv):
I think we got wrong build links. Here is the right one:
https://builds.apache.org/job/PreCommit-HDFS-Build/24325/artifact/out/diff-checkstyle-root.txt
Pasting the checkstyle warnings:
{noformat}
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:341:
AlignmentContext alignmentContext;:22: Variable 'alignmentContext' must be
private and have accessor methods. [VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:592:
/**: First sentence should end with a period. [JavadocStyle]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:610:
public static ProtocolProxy getProtocolProxy(Class protocol,:
'method def modifier' has incorrect indentation level 3, expected level should
be 2. [Indentation]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:610:
public static ProtocolProxy getProtocolProxy(Class protocol,:39:
More than 7 parameters (found 10). [ParameterNumber]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:837:
RpcResponseHeaderProto bufferedHeader;// the response header for
call:28: Variable 'bufferedHeader' must be private and have accessor methods.
[VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:838:
Writable bufferedRv; // the byte response for call:14:
Variable 'bufferedRv' must be private and have accessor methods.
[VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java:219:
public Invoker(Class protocol,:12: More than 7 parameters (found 8).
[ParameterNumber]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:35:import
org.apache.hadoop.hdfs.server.namenode.ha.IPFailoverProxyProvider;:8: Unused
import - org.apache.hadoop.hdfs.server.namenode.ha.IPFailoverProxyProvider.
[UnusedImports]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:86:
public AlignmentContextProxyProvider(:5: Redundant 'public' modifier.
[RedundantModifier]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:110:
public SpyConfiguredContextProxyProvider(:5: Redundant 'public' modifier.
[RedundantModifier]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:220:
long clientState = getContext(0).getLastSeenStateId();;:59: Empty
statement. [EmptyStatement]
{noformat}
> Make Client field AlignmentContext non-static.
> --
>
> Key: HDFS-13399
> URL: https://issues.apache.org/jira/browse/HDFS-13399
> Project: Hadoop HDFS
> Issue Type: Sub-task
>Affects Versions: HDFS-12943
>Reporter: Plamen Jeliazkov
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS-13399-HDFS-12943.000.patch,
> HDFS-13399-HDFS-12943.001.patch, HDFS-13399-HDFS-12943.002.patch,
> HDFS-13399-HDFS-12943.003.patch, HDFS-13399-HDFS-12943.004.patch,
> HDFS-13399-HDFS-12943.005.patch, HDFS-13399-HDFS-12943.006.patch,
> HDFS-13399-HDFS-12943.007.patch, HDFS-13399-HDFS-12943.008.patch,
> HDFS-13399-HDFS-12943.009.patch
>
>
> In HDFS-12977, DFSClient's constructor was altered to make use of a new
> static method in Client that allowed one to set an AlignmentContext. This
> work is to remove that static field and make each DFSClient pass it's
> AlignmentContext down to the proxy Call level.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16498592#comment-16498592
]
Konstantin Shvachko edited comment on HDFS-13399 at 6/1/18 9:20 PM:
I think we got wrong build links. Here is the right one:
https://builds.apache.org/job/PreCommit-HDFS-Build/24325/artifact/out/diff-checkstyle-root.txt
Pasting the checkstyle warnings:
{noformat}
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:341:
AlignmentContext alignmentContext;:22: Variable 'alignmentContext' must be
private and have accessor methods. [VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:592:
/**: First sentence should end with a period. [JavadocStyle]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:610:
public static ProtocolProxy getProtocolProxy(Class protocol,:
'method def modifier' has incorrect indentation level 3, expected level should
be 2. [Indentation]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:610:
public static ProtocolProxy getProtocolProxy(Class protocol,:39:
More than 7 parameters (found 10). [ParameterNumber]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:837:
RpcResponseHeaderProto bufferedHeader;// the response header for
call:28: Variable 'bufferedHeader' must be private and have accessor methods.
[VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:838:
Writable bufferedRv; // the byte response for call:14:
Variable 'bufferedRv' must be private and have accessor methods.
[VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java:219:
public Invoker(Class protocol,:12: More than 7 parameters (found 8).
[ParameterNumber]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:35:import
org.apache.hadoop.hdfs.server.namenode.ha.IPFailoverProxyProvider;:8: Unused
import - org.apache.hadoop.hdfs.server.namenode.ha.IPFailoverProxyProvider.
[UnusedImports]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:86:
public AlignmentContextProxyProvider(:5: Redundant 'public' modifier.
[RedundantModifier]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:110:
public SpyConfiguredContextProxyProvider(:5: Redundant 'public' modifier.
[RedundantModifier]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:220:
long clientState = getContext(0).getLastSeenStateId();;:59: Empty
statement. [EmptyStatement]
{noformat}
was (Author: shv):
I think we got wrong build links. Here is the right one:
https://builds.apache.org/job/PreCommit-HDFS-Build/24325/artifact/out/diff-checkstyle-root.txt
Pasting the checkstyle warnings:
{nofrmat}
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java:341:
AlignmentContext alignmentContext;:22: Variable 'alignmentContext' must be
private and have accessor methods. [VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:592:
/**: First sentence should end with a period. [JavadocStyle]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:610:
public static ProtocolProxy getProtocolProxy(Class protocol,:
'method def modifier' has incorrect indentation level 3, expected level should
be 2. [Indentation]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/RPC.java:610:
public static ProtocolProxy getProtocolProxy(Class protocol,:39:
More than 7 parameters (found 10). [ParameterNumber]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:837:
RpcResponseHeaderProto bufferedHeader;// the response header for
call:28: Variable 'bufferedHeader' must be private and have accessor methods.
[VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java:838:
Writable bufferedRv; // the byte response for call:14:
Variable 'bufferedRv' must be private and have accessor methods.
[VisibilityModifier]
./hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/WritableRpcEngine.java:219:
public Invoker(Class protocol,:12: More than 7 parameters (found 8).
[ParameterNumber]
./hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestStateAlignmentContextWithHA.java:35:import
org.apache.hadoop.hdfs.server.namenode.ha.IPFailoverProxyProvider;:8: Unused
import - org.apach
[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16494033#comment-16494033
]
Plamen Jeliazkov edited comment on HDFS-13399 at 5/29/18 6:39 PM:
--
Thanks for taking a look [~shv]!
# Removed unused import in {{NameNodeHAProxyFactory}}.
# Removed {{setAlignmentContext()}} from {{HAProxyFactory}} interface. Now only
in {{ClientHAProxyFactory}}.
# Created new method called
{{NameNodeProxiesClient.createNonHAProxyWithAlignmentContext(...,
alignmentContext)}}. I also added a conditional to only call this method if
alignmentContext is not null in {{ClientHAProxyFactory.createProxy}}. This way
there is a clear branch and use of legacy construction of the ProxyFactory
otherwise.
I would like to direct attention to {{Server.RpcCall}} changes; specifically
around the new fields {{bufferedRv}} and {{bufferedHeader}}.
In order to make AlignmentContext work while utilizing the FsEditLogAsync
implementation I needed to re-do the RPC response byte buffer construction with
a (later) modified ResponseHeader.
I encourage you to look at the {{setupResponse(RpcCall call,
RpcResponseHeaderProto header, Writable rv)}} and
{{Server.RpcCall.doResponse(Throwable t)}} methods to understand what I am
concerned about.
It would be nice to not bloat the RpcCall objects any further as NameNode is
dealing with many RPC connections all the time and this would add 2 fields
worth of heap to all {{RpcCalls}}.
I have attached .008 patch. Tested locally and unit tests passed.
was (Author: zero45):
Thanks for taking a look [~shv]!
# Removed unused import in {{NameNodeHAProxyFactory}}.
# Removed {{setAlignmentContext()}} from {{HAProxyFactory}} interface. Now only
in {{ClientHAProxyFactory}}.
# Created new method called
{{NameNodeProxiesClient.createNonHAProxyWithAlignmentContext(...,
alignmentContext)}}. I also added a conditional to only call this method if
alignmentContext is not null in {{ClientHAProxyFactory.createProxy}}. This way
there is a clear branch and use of legacy construction of the ProxyFactory
otherwise.
I would like to direct attention to {{Server.RpcCall}} changes; specifically
around the new fields {{bufferedRv}} and {{bufferedHeader}}.
In order to make AlignmentContext work while utilizing the FsEditLogAsync
implementation I needed to re-do the RPC response byte buffer construction with
a (later) modified ResponseHeader.
I encourage you to look at the {{setupResponse(RpcCall call,
RpcResponseHeaderProto header, Writable rv)}} and
{{Server.RpcCall.doResponse(Throwable t)}} methods to understand what I am
concerned about.
It would be nice to not bloat the RpcCall objects any further as NameNode is
dealing with many RPC connections all the time and this would add 2 fields
worth of heap to all {{RpcCalls}}.
> Make Client field AlignmentContext non-static.
> --
>
> Key: HDFS-13399
> URL: https://issues.apache.org/jira/browse/HDFS-13399
> Project: Hadoop HDFS
> Issue Type: Sub-task
>Affects Versions: HDFS-12943
>Reporter: Plamen Jeliazkov
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS-13399-HDFS-12943.000.patch,
> HDFS-13399-HDFS-12943.001.patch, HDFS-13399-HDFS-12943.002.patch,
> HDFS-13399-HDFS-12943.003.patch, HDFS-13399-HDFS-12943.004.patch,
> HDFS-13399-HDFS-12943.005.patch, HDFS-13399-HDFS-12943.006.patch,
> HDFS-13399-HDFS-12943.007.patch, HDFS-13399-HDFS-12943.008.patch
>
>
> In HDFS-12977, DFSClient's constructor was altered to make use of a new
> static method in Client that allowed one to set an AlignmentContext. This
> work is to remove that static field and make each DFSClient pass it's
> AlignmentContext down to the proxy Call level.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16464160#comment-16464160
]
Plamen Jeliazkov edited comment on HDFS-13399 at 5/4/18 5:21 PM:
-
Yes I propose to remove it from {{DFSClient}}. I think for now I will create a
new {{ProxyProvider}} that is only used in tests and makes use of
{{AlignmentContext}}. I will be able to pull it because I'll have access to the
instance within my tests. Similar to how others created their own {{RpcEngine}}
implementations within unit tests. This should be enough to showcase the
stateId transfer. We can remove my class if we want after we have the
{{StandbyReadsProxyProvider}} working.
Regarding the issue about the transactionId – I want to clear up below what I
am talking about:
Imagine a fresh HA-enabled DFS, at transactionId 0, is initialized. A client
connects and makes a single directory. We should now expect to be at
transactionId 1 and expect that the client received, in the RPC response
header, a stateId of 1. However this is not the case. The reason it is not the
case is because HA-enabled NameNodes utilize {{FSEditLogAsync}} which updates
the txid field, the field we rely on in
{{FSNamesystem.getLastWrittenTransactionId}}, asynchronously from the client
call. The result is that in the RPC response header the client receives a
stateId of 0. Not 1. This is clearly incorrect. We do not want a client to
connect to a NameNode that is behind in state.
Clearly this is just a race condition but it has already appeared in my unit
tests.
One idea is to modify {{FSEditLogAsync}} like so:
{code:java}
@Override
long getLastWrittenTxIdWithoutLock() {
return super.getLastWrittenTxIdWithoutLock() + editPendingQ.size() +
syncWaitQ.size();
}
{code}
However I am unsure if this would be correct / safe to do. Input from others
would be desired here.
was (Author: zero45):
Yes I propose to remove it from {{DFSClient}}. I think for now I will create a
new {{ProxyProvider}} that is only used in tests and makes use of
{{AlignmentContext}}. I will be able to pull it because I'll have access to the
instance within my tests. Similar to how others created their own {{RpcEngine}}
implementations within unit tests. This should be enough to showcase the
stateId transfer. We can remove my class if we want after we have the
{{StandbyReadsProxyProvider}} working.
Regarding the issue about the transactionId – I want to clear up below what I
am talking about:
Imagine a fresh HA-enabled DFS, at transactionId 0, is initialized. A client
connects and makes a single directory. We should now expect to be at
transactionId 1 and expect that the client received, in the RPC response
header, a stateId of 1. However this is not the case. The reason it is not the
case is because HA-enabled NameNodes utilize {{FSEditLogAsync}} which updates
the txid field, the field we rely on in
{{FSNamesystem.getLastWrittenTransactionId}}, asynchronously from the client
call. The result is that in the RPC response header the client receives they
get a stateId of 0. Not 1. This is clearly incorrect. We do not want a client
to connect to a NameNode that is behind in state.
Clearly this is just a race condition but it has already appeared in my unit
tests.
One idea is to modify {{FSEditLogAsync}} like so:
{code:java}
@Override
long getLastWrittenTxIdWithoutLock() {
return super.getLastWrittenTxIdWithoutLock() + editPendingQ.size() +
syncWaitQ.size();
}
{code}
However I am unsure if this would be correct / safe to do. Input from others
would be desired here.
> Make Client field AlignmentContext non-static.
> --
>
> Key: HDFS-13399
> URL: https://issues.apache.org/jira/browse/HDFS-13399
> Project: Hadoop HDFS
> Issue Type: Sub-task
>Affects Versions: HDFS-12943
>Reporter: Plamen Jeliazkov
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS-13399-HDFS-12943.000.patch,
> HDFS-13399-HDFS-12943.001.patch, HDFS-13399-HDFS-12943.002.patch,
> HDFS-13399-HDFS-12943.003.patch, HDFS-13399-HDFS-12943.004.patch,
> HDFS-13399-HDFS-12943.005.patch, HDFS-13399-HDFS-12943.006.patch
>
>
> In HDFS-12977, DFSClient's constructor was altered to make use of a new
> static method in Client that allowed one to set an AlignmentContext. This
> work is to remove that static field and make each DFSClient pass it's
> AlignmentContext down to the proxy Call level.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16459341#comment-16459341
]
Konstantin Shvachko edited comment on HDFS-13399 at 5/1/18 12:49 AM:
-
Spent a lot of time on this and finally realized what is the problem here.
You are trying to convert {{ConfiguredFailoverProxyProvider}} into an observer
proxy provider, which is not what we want. We want all existing proxy providers
to work exactly as they are without {{AlignmentContext}}.
And we want to create a new proxy provider, which has the
{{AlignmentContext}}. Such provider will probably use a new factory, and will
use new {{createProxy()}} methods, which will propagate {{AlignmentContext}}
into {{RPCEngine}}(s) etc. This is targeted in HDFS-12976. Until then we cannot
even build a test for that.
Here are some more thoughts:
# {{DFSClient}} should not be a holder of {{alignmentContext}} member. It
should be a part of the {{StandbyReadProxyProvider}}. Proxy providers are in
1-1 mapping with clients, so it is the same thing.
# {{alignmentContext}} in {{Invoker}} of RPC engine is probably unavoidable,
mostly because we cannot change signature of {{invoke()}} call.
# For {{ipc.Client}} it is better to move {{alignmentContext}} from {{Call}}
into {{Connection}}, and set it in {{sendRpcRequest(Call, AlignmentContext)}}
rather than in the constructor. I can send you a patch for that.
I don't know if 2 and 3 belong to this jira or to HDFS-12976. In any case
removing static {{AlignmentContext}} from {{Client}} still makes sense. Only
your tests will not work until {{StandbyReadProxyProvider}} is introduced. Hold
on to them until then.
was (Author: shv):
Spent a lot of time on this and finally realized what is the problem here.
You are trying to convert {{ConfiguredFailoverProxyProvider}} into an observer
proxy provider, which is not what we want. We want all existing proxy providers
to work exactly as they are without {{AlignmentContext}}.
And we want to create a new proxy provider, which has the {{AlignmentContext}}.
Such provider will probably use a new factory, and will use new
{{createProxy()}} methods, which will propagate {{AlignmentContext}} into
{{RPCEngine}}(s) etc. This is targeted in HDFS-12976. Until then we cannot even
build a test for that.
Here are some more thoughts:
# {{DFSClient}} should not be a holder of {{alignmentContext}} member. It
should be a part of the {{StandbyReadProxyProvider}}. Proxy providers are in
1-1 mapping with clients, so it is the same thing.
# {{alignmentContext}} in {{Invoker}} of RPC engine is probably unavoidable,
mostly because we cannot change signature of {{invoke()}} call.
# For {{ipc.Client}} it is better to move {{alignmentContext}} from {{Call}}
into {{Connection}}, and set it in sendRpcRequest(Call, AlignmentContext)}}
rather than in the constructor. I can send you a patch for that.
I don't know if 2 and 3 belong to this jira or to HDFS-12976. In any case
removing static {{AlignmentContext}} from {{Client}} still makes sense. Only
your tests will not work until {{StandbyReadProxyProvider}} is introduced. Hold
on to them until then.
> Make Client field AlignmentContext non-static.
> --
>
> Key: HDFS-13399
> URL: https://issues.apache.org/jira/browse/HDFS-13399
> Project: Hadoop HDFS
> Issue Type: Sub-task
>Affects Versions: HDFS-12943
>Reporter: Plamen Jeliazkov
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS-13399-HDFS-12943.000.patch,
> HDFS-13399-HDFS-12943.001.patch, HDFS-13399-HDFS-12943.002.patch,
> HDFS-13399-HDFS-12943.003.patch, HDFS-13399-HDFS-12943.004.patch,
> HDFS-13399-HDFS-12943.005.patch
>
>
> In HDFS-12977, DFSClient's constructor was altered to make use of a new
> static method in Client that allowed one to set an AlignmentContext. This
> work is to remove that static field and make each DFSClient pass it's
> AlignmentContext down to the proxy Call level.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16451323#comment-16451323
]
Konstantin Shvachko edited comment on HDFS-13399 at 4/24/18 10:42 PM:
--
There is some misunderstanding. Why do you still need
{{createNonHAProxyWithClientProtocol()}} and {{createNonHAProxy()}} with
{{AlignmentContext}} as an argument? I thought you will revert this.
was (Author: shv):
There is some misunderstanding. Why do you still need
{{createNonHAProxyWithClientProtocol()}} with {{AlignmentContext}} as an
argument? I thought you will revert this.
> Make Client field AlignmentContext non-static.
> --
>
> Key: HDFS-13399
> URL: https://issues.apache.org/jira/browse/HDFS-13399
> Project: Hadoop HDFS
> Issue Type: Sub-task
>Affects Versions: HDFS-12943
>Reporter: Plamen Jeliazkov
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS-13399-HDFS-12943.000.patch,
> HDFS-13399-HDFS-12943.001.patch, HDFS-13399-HDFS-12943.002.patch,
> HDFS-13399-HDFS-12943.003.patch, HDFS-13399-HDFS-12943.004.patch
>
>
> In HDFS-12977, DFSClient's constructor was altered to make use of a new
> static method in Client that allowed one to set an AlignmentContext. This
> work is to remove that static field and make each DFSClient pass it's
> AlignmentContext down to the proxy Call level.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (HDFS-13399) Make Client field AlignmentContext non-static.
[
https://issues.apache.org/jira/browse/HDFS-13399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16433151#comment-16433151
]
Plamen Jeliazkov edited comment on HDFS-13399 at 4/10/18 11:24 PM:
---
Hey [~xkrogen], thanks for the review! Here's what I found:
(1) I am not sure if that Javadoc link error is related to my changes. I did
not modify {{NameNodeProxies#createProxy(Configuration, URI, Class)}} and with
my changes shelved / removed I still see the Javadoc link error on
{{NameNodeProxiesClient#createProxyWithClientProtocol}}. Unless you are
referring to something else I am not sure what I missed...
(2) I agree they appear related. Took a quick look. It seems
{{Client.createCall(RPC.RpcKind rpcKind, Writable rpcRequest)}} is now unused
in favor of the overloaded method that adds {{AlignmentContext}} as a
parameter. Should I just delete {{Client.createCall(RPC.RpcKind rpcKind,
Writable rpcRequest)}} or should I modify TestIPC and TestAsyncIPC to call the
new method? I would vote to remove the two parameter method and update the
tests as it really seems it is just unused now.
(3) I will apply your changes in a next patch; I also have made some test
optimizations on my own as well – mostly around removing unneeded
{{cluster.waitActive()}} calls.
As for the configuration question – it was more-so just out of curiosity to see
if you guys wanted this to just be a standard feature of {{DFSClients}} and
{{FSNamesystem}} going forward or if you wanted to try to preserve the legacy
behavior. I agree overhead is very minimal; I would say in both processing and
payload. I can only think it may be useful to revert to "old behavior" in the
event that there is ever some crippling bug in the client or server side of the
RPC processing. I would vote to make it a standard feature, but again, I wanted
to ask and see your opinions.
was (Author: zero45):
(1) I am not sure if that Javadoc link error is related to my changes. I did
not modify {{NameNodeProxies#createProxy(Configuration, URI, Class)}} and with
my changes shelved / removed I still see the Javadoc link error on
{{NameNodeProxiesClient#createProxyWithClientProtocol}}. Unless you are
referring to something else I am not sure what I missed...
(2) I agree they appear related. Took a quick look. It seems
{{Client.createCall(RPC.RpcKind rpcKind, Writable rpcRequest)}} is now unused
in favor of the overloaded method that adds {{AlignmentContext}} as a
parameter. Should I just delete {{Client.createCall(RPC.RpcKind rpcKind,
Writable rpcRequest)}} or should I modify TestIPC and TestAsyncIPC to call the
new method? I would vote to remove the two parameter method and update the
tests as it really seems it is just unused now.
(3) I will apply your changes in a next patch; I also have made some test
optimizations on my own as well – mostly around removing unneeded
{{cluster.waitActive()}} calls.
As for the configuration question – it was more-so just out of curiosity to see
if you guys wanted this to just be a standard feature of {{DFSClients}} and
{{FSNamesystem}} going forward or if you wanted to try to preserve the legacy
behavior. I agree overhead is very minimal; I would say in both processing and
payload. I can only think it may be useful to revert to "old behavior" in the
event that there is ever some crippling bug in the client or server side of the
RPC processing. I would vote to make it a standard feature, but again, I wanted
to ask and see your opinions.
> Make Client field AlignmentContext non-static.
> --
>
> Key: HDFS-13399
> URL: https://issues.apache.org/jira/browse/HDFS-13399
> Project: Hadoop HDFS
> Issue Type: Sub-task
>Affects Versions: HDFS-12943
>Reporter: Plamen Jeliazkov
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS-13399-HDFS-12943.000.patch,
> HDFS-13399-HDFS-12943.001.patch
>
>
> In HDFS-12977, DFSClient's constructor was altered to make use of a new
> static method in Client that allowed one to set an AlignmentContext. This
> work is to remove that static field and make each DFSClient pass it's
> AlignmentContext down to the proxy Call level.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
