hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Douglas (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-11546) Federation Router RPC server
Date Wed, 12 Apr 2017 01:06:41 GMT

    [ https://issues.apache.org/jira/browse/HDFS-11546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15965226#comment-15965226

Chris Douglas commented on HDFS-11546:

Looks good, overall. A few questions:
* Many fields e.g., {{ConnectionManager.Timer}}, {{RouterRpcClient.retryPolicy}}, {{RouterRpcClient.executorService}}
can be final
* {{RemoteLocationContext#equals}} relying on hashcode equality of Strings is too weak
* In {{ConnectionPool#getNumActiveConnections}}, instead of catching AIOOBException: use a
lock, COWArrayList, or other threadsafe collection.
* The threading in {{ConnectionPool}} generally seems wonky. It avoids locks, but catches
a lot of exceptions. Instead of synchronizing on a special {{Object lock}}, most of this could
synchronize on the {{connections}} field or use a threadsafe collections.
* {{ConnectionPool}} seems to distribute requests to connections RR, adding new connections
(up to some limit) if it wraps while a connection is still "busy". Is that right?
* {{ConnectionPool#cleanup}} should remove from the end of an ArrayList to avoid copies, not
the front (and be correctly synchronized)
* If the client in the {{ConnectionPool}} is configured to retry, and the proxied client retries,
that may go to a different connection in the pool (or a different router), right? Should the
proxy never retry to avoid repeating past operations, or does some other mechanism prevent
* {{ConnectionPool#close}} doesn't seem to do any cleanup work (interrupting threads, etc.).
This is a "soft" shutdown?
* {{ConnectionManager}} uses {{Timer/TimerTask}}, which are sort-of deprecated in favor of
{{ScheduledThreadPoolExecutor}} after 1.5
* The {{ConnectionManager}} creates keys for each RPC proxy by creating a String of the UGI
and hashcode for each token. The chance of collision seems remote, but unnecessarily non-zero.
Instead of a flat {{String -> ConnectionPool}} map, could this maintain a key of user/NN?
Is there a particular reason to include tokens as part of the key?
* Does {{RouterRpcClient#invokeSequential}} and {{RouterRpcClient#invokeConcurrent}} implement
functionality similar to HADOOP-12077? There should probably be a documentation JIRA to describe
common patterns, limitations, and deployment. In particular, the subset of {{ClientProtocol}}
implemented by {{RouterRpcServer}} should be documented.
* I didn't review the details of {{RouterRpcServer}} since most of it seems to be wrapping
the client proxy, but there are some TODO items there. Those don't need to block commit to
the branch, but they should probably be documented or addressed before merge.
* The defaults in {{hdfs-site.xml}} look conservative; the description could include some
cursory guidance on setting them.
* Thanks for adding the integration tests

> Federation Router RPC server
> ----------------------------
>                 Key: HDFS-11546
>                 URL: https://issues.apache.org/jira/browse/HDFS-11546
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: hdfs
>    Affects Versions: HDFS-10467
>            Reporter: Inigo Goiri
>            Assignee: Inigo Goiri
>         Attachments: HDFS-11546-HDFS-10467-000.patch, HDFS-11546-HDFS-10467-001.patch,
HDFS-11546-HDFS-10467-002.patch, HDFS-11546-HDFS-10467-003.patch, HDFS-11546-HDFS-10467-004.patch
> RPC server side of the Federation Router implements ClientProtocol.

This message was sent by Atlassian JIRA

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

View raw message