[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130903#comment-17130903
]
Yiqun Lin commented on HDFS-15346:
----------------------------------
[~LiJinglun] , thanks for addressing the comments, almost looks good now.
{quote}Agree with you ! Using a fedbalance-default.xml is much better.
{quote}
Would you create a subtask JIRA for this? Let's try to complete this in a later
time.
{quote}I'll try to figure it out. But it might be quite tricky as the unit
tests use both MiniDFSCluster and MiniMRYarnCluster. And there are many rounds
of distcp. Please tell me if you have any suggestions, thanks
{quote}
I will take a further look for this later. But anyway, currently the unit tests
can all be passed, it's okay for me.
Still some remaining minor comments:
*hadoop-federation-balance/pom.xml*
{noformat}
+ <dependency>
+ <groupId>org.bouncycastle</groupId>
+ <artifactId>bcprov-jdk15on</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.bouncycastle</groupId>
+ <artifactId>bcpkix-jdk15on</artifactId>
+ <scope>test</scope>
+ </dependency>
{noformat}
These two dependencies seems not related, can we remove this one?
*DistCpFedBalance.java/FedBalance.java*
I don't know why we define another class FedBalance. This FedBalance can just
combined to DistCpFedBalance. I prefer to override main method in
DistCpFedBalance and then renamed DistCpFedBalance to FedBalance.
*DistCpBalanceOptions.java*
Find two places can be described more clear:
# I prefer to move detailed comment message into option description and users
can known detailed about this option.
{code:java}
/**
* Run in router-based federation mode.
*/
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 read and write by
cancelling all permissions of the source path. The default value
is `false`.");
{code}
# The description of delay option is hard to understand. I make a minor change
for this. [~LiJinglun], if you have a better description for this option, feel
free to update your change on this.
{code:java}
/* Specify the delayed duration(millie seconds) to recover the Job.*/
final static Option DELAY_DURATION = new Option("delay", true,
"The delayed duration(millie seconds) to recover the Job continue to run
when the job is detected that it hasn't been finished and waits to complete.");
{code}
*DistCpProcedure.java*
# Move {{srcFs.allowSnapshot(src);}} to at the end of method. Only after the
snapshot check, then we do the allow snapshot opertion.
{code:java}
+
+ private void cleanUpBeforeInitDistcp() throws IOException {
+ if (dstFs.exists(dst)) { // clean up.
+ throw new IOException("The dst path=" + dst + " already exists. The
admin"
+ + " should delete it before submitting the initial distcp job.");
+ }
+ Path snapshotPath = new Path(src,
+ HdfsConstants.DOT_SNAPSHOT_DIR_SEPARATOR + CURRENT_SNAPSHOT_NAME);
+ if (srcFs.exists(snapshotPath)) {
+ throw new IOException("The src snapshot=" + snapshotPath +
+ " already exists. The admin should delete the snapshot before"
+ + " submitting the initial distcp.");
+ }
srcFs.allowSnapshot(src); <--- move to here
+ }
{code}
*FedBalanceContext.java*
# Please add necessary dot in toString method, like this:
{code:java}
public String toString() {
StringBuilder builder = new StringBuilder("FedBalance context:");
builder.append(" src=").append(src);
builder.append(", dst=").append(dst);
if (useMountReadOnly) {
builder.append(", router-mode=true");
builder.append(", mount-point=").append(mount);
} else {
builder.append(", router-mode=false");
}
builder.append(", forceCloseOpenFiles=").append(forceCloseOpenFiles);
builder.append(", trash=").append(trashOpt.name());
builder.append(", map=").append(mapNum);
builder.append(", bandwidth=").append(bandwidthLimit);
return builder.toString();
}
{code}
# Can you add new added option delayDuration option into this class?
> RBF: DistCpFedBalance implementation
> ------------------------------------
>
> Key: HDFS-15346
> URL: https://issues.apache.org/jira/browse/HDFS-15346
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Reporter: Jinglun
> Assignee: Jinglun
> Priority: Major
> Attachments: HDFS-15346.001.patch, HDFS-15346.002.patch,
> HDFS-15346.003.patch, HDFS-15346.004.patch, HDFS-15346.005.patch,
> HDFS-15346.006.patch, HDFS-15346.007.patch, HDFS-15346.008.patch,
> HDFS-15346.009.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This
> is the second one. Detail can be found at HDFS-15294.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
