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

Íñigo Goiri commented on HDFS-13776:
------------------------------------

Thanks [~dibyendu_hadoop] for  [^HDFS-13776-003.patch].
Integrating with SPS is very valuable and surfacing to the Router can be pretty 
powerful.

A few comments:
* I think we should tackle the checkstyles.
* In {{RouterStoragePolicy}}, we should be consistent with other like 
{{ErasureCoding}} and make the javadoc comments in the fields as one liners 
(e.g., {{/** RPC server to receive client calls. */}}).
* We should add a javadoc to {{MiniRouterDFSCluster#setStorageTypes()}}.
* What happens if {{storageTypes}} is left to null? I'm guessing MiniDFSCluster 
behaves correctly.
* Can we add a comment in {{testProxyGetAndUnsetStoragePolicy()}} explaining 
why we look for a policy without copy on create?
* In {{TestRouterRpc#testProxyGetAndUnsetStoragePolicy()}} we should also add 
one check to compare with the namenode output; for example, we could do the 
{{getStoragePolicies()}} or {{getStoragePolicy()}}. The rest of the class does 
such checks.
* A new unit test for the {{StoragePolicySatisfier}} makes it easier to follow 
but it also brings the new creation of a {{MiniRouterDFSCluster}} 
(surprisingly, this new test is only 11 seconds to run, but much slower than 
the others with <1 sec). Is it simple to add it to {{TestRouterRpc}}? 
Otherwise, 11 seconds is good enough.
* In {{testStoragePolicySatisfier()}} can we compare with the NN outputs?

> RBF: Add Storage policies related ClientProtocol APIs
> -----------------------------------------------------
>
>                 Key: HDFS-13776
>                 URL: https://issues.apache.org/jira/browse/HDFS-13776
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Dibyendu Karmakar
>            Assignee: Dibyendu Karmakar
>            Priority: Major
>         Attachments: HDFS-13776-000.patch, HDFS-13776-001.patch, 
> HDFS-13776-002.patch, HDFS-13776-003.patch
>
>
> Currently unsetStoragePolicy and getStoragePolicy are not implemented in 
> RouterRpcServer.



--
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

Reply via email to