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

Yiqun Lin commented on HDFS-15410:
----------------------------------

Besides [~elgoiri]'s review comment, some more reivew comments from me:

Not fully understand why we need to define the class impl config to do 
reflection and get the instance. Currently there is no other implement class, 
why not just create new FedBalance/BalanceJournalInfoHDFS instance in the code? 
From my understanding, this two config settings is can be removed.
{code:java}
federation.balance.class
hadoop.hdfs.procedure.journal.class

    // init journal.
    Class<BalanceJournal> clazz = (Class<BalanceJournal>) conf
        .getClass(JOURNAL_CLASS, BalanceJournalInfoHDFS.class);
    journal = ReflectionUtils.newInstance(clazz, conf);

    Class<Tool> balanceClazz = (Class<Tool>) conf
        .getClass(FEDERATION_BALANCE_CLASS, FedBalance.class);
    Tool balancer = ReflectionUtils.newInstance(balanceClazz, conf);
{code}

Can we rename class name from {{DistCpBalanceOptions}} to 
{{FedBalanceOptions}}? This will look more readable that these options here are 
making sense for fedbalance tool.

Can we rename config prefix from {{hadoop.hdfs.procedure.work.thread.num}} to 
{{hdfs.fedbalance.procedure.work.thread.num}}?

Following description need to be updated here since -router option doesn't 
require to inout true or false as a parameter now.
{noformat}
  final static Option ROUTER = new Option("router", false,
      "If `true` the command runs in router mode. The source path is "
          + "taken as a mount point. It will disable write by setting the mount"
          + " point readonly. Otherwise the command works in normal federation"
          + " mode. The source path is taken as the full path. It will disable"
          + " write by cancelling all permissions of the source path. The"
          + " default value is `true`.");
{noformat}

> Add separated config file fedbalance-default.xml for fedbalance tool
> --------------------------------------------------------------------
>
>                 Key: HDFS-15410
>                 URL: https://issues.apache.org/jira/browse/HDFS-15410
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Jinglun
>            Assignee: Jinglun
>            Priority: Major
>         Attachments: HDFS-15410.001.patch, HDFS-15410.002.patch
>
>
> Add a separated config file named fedbalance-default.xml for fedbalance tool 
> configs. It's like the ditcp-default.xml for distcp tool.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to