Erik Krogen commented on HDFS-13399:

Few comments on the v001 patch:
* Looks like the Javadoc for 
{{NameNodeProxiesClient#createProxyWithClientProtocol}} has a link error.
* Test(Async)IPC failures look like they are probably relevant, can you 
* {{TestStateAlignmentContextWithHA}} looks good, but I think can be improved a 
** If the {{assertThat}} statement throws something, the thread will die, but 
the exception will not be bubbled up to the main thread. It is probably better 
instead to have an {{AtomicBoolean}} that is set to some value if a thread sees 
a failing condition, and check for this in the main thread.
** I think you can do something more clean than the current looping approach to 
wait for the threads to exit. Maybe use an {{ExecutorService}} with fixed 
thread pool, {{invokeAll}}, then {{shutdown}}, followed by repeated calls to 
{{awaitTermination(1, TimeUnit.SECONDS)}}. With {{ExecutorService}} you can 
also use a {{Callable}} instead of {{Runnable}} and then have each thread 
return a special value if it encountered an unexpected condition (doing away 
with the {{AtomicBoolean}} mentioned above).
** You have quite a few instances of missing a space between e.g. 
{{for}}/{{while}} and an opening parenthesis.

Should we consider creating client and server side configuration for enabling / 
disabling AlignmentContext processing?
Is there any advantage to turning it off? Is there any additional overhead? The 
only thing I can think of is increasing the RPC payload size slightly.

> 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

To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to