[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17135841#comment-17135841
]
Hadoop QA commented on HDFS-15346:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
49s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
8s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 24m
15s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 21m
12s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 3m
16s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
17m 39s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 4m
53s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 6m
30s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
31s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
34s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
33s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
32s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 5m
30s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 20m
58s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 20m
58s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
3m 19s{color} | {color:orange} root: The patch generated 5 new + 2 unchanged -
0 fixed = 7 total (was 2) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 7m
5s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
0s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} shelldocs {color} | {color:green} 0m
32s{color} | {color:green} There were no new shelldocs issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
6s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
17m 25s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
29s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
30s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
34s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {colo
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17135819#comment-17135819
]
Yiqun Lin commented on HDFS-15346:
--
[~LiJinglun], the refactor looks great. I find you decrease the timeout value,
the new value seems too small and it will lead timeout error.
Can you adjust all this time value to 3(@Test(timeout = 3) in
TestDistCpProcedure? This value works well in my local.
Finally, can we add 'fedbalance' in current package name under fedbalance
module?
Under module path src/test/java, src/main/java
Update
{noformat}
org.apache.hadoop.tools
org.apache.hadoop.tools.procedure
{noformat}
to
{noformat}
org.apache.hadoop.tools.fedbalance
org.apache.hadoop.tools.fedbalance.procedure
{noformat}
Then please check and update some old class path that used in the module, like
hadoop-federation-balance.sh, pom.xml or some other place.
Others looks good to me now. Thanks [~LiJinglun] for the so patient working for
this.
Once above are addressed, I will hold off the commit for few days in case there
are some other comments from others.
> 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, HDFS-15346.010.patch, HDFS-15346.011.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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17135615#comment-17135615
]
Jinglun commented on HDFS-15346:
Your are genius [~linyiqun] ! Thanks your brilliant comments, the improvement
is great ! The unit tests run very fast now. I followed up all the changes. And
I did a little refactor based on your improvement. The logic of the improvement
is the same as you suggested. I only extracted a method and refactored the
class RunningJobStatus to make it easier to read. Please let me know your
thoughts, I'm also ok to keep it just the same as you suggested.
{quote}Can you update following description in router option? I update this
content as well but seems this was not addressed in the latest patch.
{quote}
Sorry I missed this. Update at v11.
{quote}Method name cleanUpBeforeInitDistcp can be renamed to
pathCheckBeforeInitDistcp since we don't do any cleanup operation now.
{quote}
Address at v11.
> 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, HDFS-15346.010.patch, HDFS-15346.011.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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17135024#comment-17135024
]
Yiqun Lin commented on HDFS-15346:
--
[~LiJinglun], thanks for addressing remaining comments.
These two days, I am trying to improve the efficiency of the unit test, current
unit test is too slow.
I find another way that we don't have to depend on mini yarn cluster in test
running. The job can submitted and executed in LocalJobRunner way. But we need
to make an adjustment in getting job status from job client.
I do some refactor in getCurrent method and apply them in DistCpProcedure.
Following are part of some necessary change we need to update.
{noformat}
@VisibleForTesting
private Job runningJob;
static boolean ENABLED_FOR_TEST = false;
...
private String submitDistCpJob(String srcParam, String dstParam,
boolean useSnapshotDiff) throws IOException {
...
try {
LOG.info("Submit distcp job={}", job);
runningJob = job; <--- need to reset there
return job.getJobID().toString();
} catch (Exception e) {
throw new IOException("Submit job failed.", e);
}
}
private RunningJobStatus getCurrentJob() throws IOException {
if (jobId != null) {
if (ENABLED_FOR_TEST) {
if (this.runningJob != null) {
Job latestJob = null;
try {
latestJob = this.runningJob.getCluster()
.getJob(JobID.forName(jobId));
} catch (InterruptedException e) {
throw new IOException(e);
}
return latestJob == null ? null
: new RunningJobStatus(latestJob, null);
}
} else {
RunningJob latestJob = client.getJob(JobID.forName(jobId));
return latestJob == null ? null :
new RunningJobStatus(null, latestJob);
}
}
return null;
}
class RunningJobStatus {
Job testJob;
RunningJob job;
public RunningJobStatus(Job testJob, RunningJob job) {
this.testJob = testJob;
this.job = job;
}
String getJobID() {
return ENABLED_FOR_TEST ? testJob.getJobID().toString()
: job.getID().toString();
}
boolean isComplete() throws IOException {
return ENABLED_FOR_TEST ? testJob.isComplete() : job.isComplete();
}
boolean isSuccessful() throws IOException {
return ENABLED_FOR_TEST ? testJob.isSuccessful() : job.isSuccessful();
}
String getFailureInfo() throws IOException {
try {
return ENABLED_FOR_TEST ? testJob.getStatus().getFailureInfo()
: job.getFailureInfo();
} catch (InterruptedException e) {
throw new IOException(e);
}
}
}
{noformat}
And mini yarn cluster related code lines can all be removed (include two pom
dependencies mentioned above)
{code:java}
+mrCluster = new MiniMRYarnCluster(TestDistCpProcedure.class.getName(), 3);
+conf.set(MRJobConfig.MR_AM_STAGING_DIR, "/apps_staging_dir");
+mrCluster.init(conf);
+mrCluster.start();
+conf = mrCluster.getConfig();
{code}
We need additionally set test enabled flag.
{code:java}
public static void beforeClass() throws IOException {
DistCpProcedure.ENABLED_FOR_TEST = true;
...
}
{code}
After this improvement, the whole test runs very faster than before, it totally
costs less than 1 min.
Also I catch some places still needed to update.
# Can you update following description in router option? I update this content
as well but seems this was not addressed in the latest patch.
{noformat}
It will disable read and write by cancelling all permissions of the source
path. The default value is `false`."
{noformat}
# Method name cleanUpBeforeInitDistcp can be renamed to
pathCheckBeforeInitDistcp since we don't do any cleanup operation now.
> 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, HDFS-15346.010.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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17134195#comment-17134195
]
Hadoop QA commented on HDFS-15346:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 2m
11s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
11s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 25m
30s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 18m
13s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 5m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
16m 44s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 4m
36s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 6m
21s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
30s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
30s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
31s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
31s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 5m
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 19m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 19m
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
1s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} shelldocs {color} | {color:green} 0m
34s{color} | {color:green} There were no new shelldocs issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
7s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
16m 25s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
26s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
31s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-tools/hadoop-too
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17134069#comment-17134069 ] Jinglun commented on HDFS-15346: Hi [~linyiqun], thanks your great comments ! Follow all your advices. Only the dependency *org.bouncycastle* is used by the MiniMRYarnCluster. Remove it will break the TestDistCpProcedure so I keep it. > 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, HDFS-15346.010.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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
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}
+
+ org.bouncycastle
+ bcprov-jdk15on
+ test
+
+
+ org.bouncycastle
+ bcpkix-jdk15on
+ test
+
{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 mes
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130351#comment-17130351
]
Hadoop QA commented on HDFS-15346:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
0s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
1s{color} | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
18s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m
48s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 21m
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 3m
24s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
53s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
17m 24s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
26s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 6m
23s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
38s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
38s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
42s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
33s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 5m
32s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 20m
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 20m
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 3m
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 7m
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
0s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} shelldocs {color} | {color:green} 0m
45s{color} | {color:green} There were no new shelldocs issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
6s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
14m 16s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 6m
41s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
45s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
44s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
46s{color} | {color:blue} hadoop-tools/hadoop-t
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130008#comment-17130008 ] Jinglun commented on HDFS-15346: Upload v09, fix unit test. > 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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129686#comment-17129686
]
Hadoop QA commented on HDFS-15346:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 27m
3s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | {color:green} No case conflicting files found. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
20s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m
15s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 18m
13s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 5m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 44s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 4m
38s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 5m
20s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
33s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
33s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
34s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
31s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
53s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 17m
49s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 17m
49s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
46s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
0s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} shelldocs {color} | {color:green} 0m
36s{color} | {color:green} There were no new shelldocs issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
6s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 48s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
20s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
33s{color} | {color:blue} hadoop-tools/hadoop-too
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17129283#comment-17129283 ] Jinglun commented on HDFS-15346: Upload v08. All the comments are fixed except the 'read only in normal federation' and the 'optimization of unit tests'. Pending jenkins. > 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 > > > 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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17128305#comment-17128305
]
Jinglun commented on HDFS-15346:
Hi [~linyiqun], thanks your great comments and valuable suggestions ! I'll
need some time to shoot all of them. So let me respond to the question first.
{quote}Here we reset permission to 0, that means no any operation is allowed?
Is this expected, why not is 400 (only allow read)? The comment said that
'cancelling the x permission of the source path.' makes me confused.
{quote}
Yes here we reset the permission to 0. Both read and write in the source path
and all its sub-paths are denied. As far as I know all the read operations need
to check its parents' execution permission. So setting to 400 can't make it
only allowing read. We still can't read its sub-paths. I think the only way to
make it 'only allowing read' is to recursively reduce each directory's
permission to 555. Reduce permission means: if the original permission is 777
then change it to 555. If the original permission is 700 then make it to 500.
Saving all the directories' permissions is very expensive. A better way may be
letting the NameNode to support 'readonly-directory'. I think we can first
using the '0 permission' way to make sure the data is consistent. Then start a
sub-task to enable the NameNode 'readonly-directory'. Finally change this to
the NameNode 'readonly-directory'.
> 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
>
>
> 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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17127498#comment-17127498
]
Yiqun Lin commented on HDFS-15346:
--
Review comments for unit tests:
*TestDistCpProcedure.java*
# Use CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY to replace
'fs.defaultFS'.
# In {{TestDistCpProcedure#testSuccessfulDistCpProcedure}}, can we add
additional file length check between src file and dst file?
# Please complete the javadoc comment for method executeProcedure and
createFiles.
# Method sede can update to a more readable name serializeProcedure.
# I think we missing a corner case test case that disable writer behavior in
non-RBF mode.
# The test need a little long time to execute the whole test.
>From Jenkins test result:
{noformat}
testDiffDistCp 1 min 18 secPassed
testInitDistCp 22 sec Passed
testRecoveryByStage 55 sec Passed
testShutdown8.9 sec Passed
testStageFinalDistCp47 sec Passed
testStageFinish 0.22 secPassed
testSuccessfulDistCpProcedure 38 sec Passed
{noformat}
Can we look into why some ut spend so many time? Increasing timeout value is a
quick-fix way but not the best way.
*TestMountTableProcedure.java*
Please update testSeDe to testSeDeserialize
*TestTrashProcedure.java*
Can we also add a test method testSeDeserialize like TestMountTableProcedure
does?
> 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
>
>
> 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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17127489#comment-17127489
]
Yiqun Lin commented on HDFS-15346:
--
Some more detailed review comments:
*HdfsConstants.java*
Can we rename DOT_SNAPSHOT_SEPARATOR_DIR to the more readable name
DOT_SNAPSHOT_DIR_SEPARATOR?
*DistCpFedBalance.java*
# It would good to print the fed context that created from input options, so
that we will know final options that we passed in.
{noformat}
+. // --> print fed balancer context
+ // Construct the balance job.
+ BalanceJob.Builder builder = new
BalanceJob.Builder<>();
+ DistCpProcedure dcp =
+ new DistCpProcedure(DISTCP_PROCEDURE, null, delayDuration, context);
+ builder.nextProcedure(dcp);
{noformat}
# We can replace this system out in LOG instance,
{noformat}
+for (BalanceJob job : jobs) {
+ if (!job.isJobDone()) {
+unfinished++;
+ }
+ System.out.println(job);
+}
{noformat}
*DistCpProcedure.java*
# The message in IOException(src + " doesn't exist.") not correctly described,
should be 'src + " should be the directory."'
# For each stage change, can we add aN additional output log, like this:
{noformat}
+if (srcFs.exists(new Path(src, HdfsConstants.DOT_SNAPSHOT_DIR))) {
+ throw new IOException(src + " shouldn't enable snapshot.");
+}
LOG.info("Stage updated from {} to {}.", stage.name(),
Stage.INIT_DISTCP.name())
+stage = Stage.INIT_DISTCP;
+ }
{noformat}
# Here we reset permission to 0, that means no any operation is allowed? Is
this expected, why not is 400 (only allow read)? The comment said that
'cancelling the x permission of the source path.' makes me confused.
{noformat}
srcFs.setPermission(src, FsPermission.createImmutable((short) 0));
{noformat}
# I prefer to throw IOException rather than doing delete operation in
cleanUpBeforeInitDistcp. cleanUpBeforeInitDistcp is expected to be the final
pre-check function before submit ditcp job.
{noformat}
+ private void initialCheckBeforeInitDistcp() throws IOException {
+if (dstFs.exists(dst)) {
+ throw IOException();
+}
+srcFs.allowSnapshot(src);
+if (srcFs.exists(new Path(src,
+HdfsConstants.DOT_SNAPSHOT_SEPARATOR_DIR + CURRENT_SNAPSHOT_NAME))) {
throw IOException();
+}
{noformat}
*FedBalanceConfigs.java*
Can we move all keys from BalanceProcedureConfigKeys to this class? We don't
need two duplicated Config class. One follow-up task I am thinking that we can
have a separated config file something named fedbalance-default.xml for
fedbalance tool, like ditcp-default.xml for distcp tool now. I don't prefer to
add all tool config settings into hdfs-default.xml.
*FedBalanceContext.java*
Override the toString method in FedBalanceContext to help us know the input
options that actually be used.
*MountTableProcedure.java*
The for loop can just break once we find the first source path that matched.
{noformat}
+for (MountTable result : results) {
+ if (mount.equals(result.getSourcePath())) {
+ existingEntry = result;
break;
+ }
+}
{noformat}
*TrashProcedure.java*
{noformat}
+ /**
+ * Delete source path to trash.
+ */
+ void moveToTrash() throws IOException {
+Path src = context.getSrc();
+if (srcFs.exists(src)) {
+ switch (context.getTrashOpt()) {
+ case TRASH:
+conf.setFloat(FS_TRASH_INTERVAL_KEY, 1);
+if (!Trash.moveToAppropriateTrash(srcFs, src, conf)) {
+ throw new IOException("Failed move " + src + " to trash.");
+}
+break;
+ case DELETE:
+if (!srcFs.delete(src, true)) {
+ throw new IOException("Failed delete " + src);
+}
+LOG.info("{} is deleted.", src);
+break;
+ default:
+break;
+ }
+}
+ }
{noformat}
For above lines, two review comments:
# Can we add SKIP option check as well and throw unexpected option error?
{noformat}
case SKIP:
break;
+ default:
+ throw new IOException("Unexpected trash option=" +
context.getTrashOpt());
+ }
{noformat}
# FS_TRASH_INTERVAL_KEY defined with 1 is too small, that means we the trash
will be deleted after 1 minute. Can you increased this to 60? Also please add
necessary comment in trash option description to say the default trash behavior
when trash is disabled in server side and client side value will be used.
> 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,
> H
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17126332#comment-17126332 ] Yiqun Lin commented on HDFS-15346: -- Will give detailed review on this weekend, [~LiJinglun]. > 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 > > > 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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17125790#comment-17125790 ] Jinglun commented on HDFS-15346: Hi [~linyiqun], v07 has passed the tests. Could you help to review it. Thanks very much ! > 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 > > > 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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17124637#comment-17124637
]
Hadoop QA commented on HDFS-15346:
--
| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
50s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
1s{color} | {color:green} No case conflicting files found. {color} |
| {color:blue}0{color} | {color:blue} shelldocs {color} | {color:blue} 0m
0s{color} | {color:blue} Shelldocs was not available. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
14s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 22m
1s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 18m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
54s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 5m
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 42s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 4m
40s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 5m
19s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
30s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
51s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 17m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 17m
34s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
41s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
1s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
1s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
6s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 52s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
19s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
34s{color} | {color:blue} hadoop-tools/hadoop-tools-dist ha
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17124152#comment-17124152
]
Hadoop QA commented on HDFS-15346:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 1m
26s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
0s{color} | {color:green} No case conflicting files found. {color} |
| {color:blue}0{color} | {color:blue} shelldocs {color} | {color:blue} 0m
1s{color} | {color:blue} Shelldocs was not available. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
4s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 21m
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 18m
0s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 2m
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 5m
57s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 50s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 4m
38s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 5m
20s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
31s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
30s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
49s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 17m
26s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 17m
26s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
2m 54s{color} | {color:orange} root: The patch generated 12 new + 2 unchanged -
0 fixed = 14 total (was 2) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
0s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
6s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 53s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
17s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
33s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17123824#comment-17123824 ] Jinglun commented on HDFS-15346: Upload v06 fix checkstyle and unit tests. > 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 > > > 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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[ https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17123467#comment-17123467 ] Yiqun Lin commented on HDFS-15346: -- [~LiJinglun], can you fix related failure ut and generated checkstyle warnings? The patch generated 19 new + 2 unchanged - 0 fixed = 21 total (was 2) > 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 > > > 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: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17123441#comment-17123441
]
Hadoop QA commented on HDFS-15346:
--
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 21m
20s{color} | {color:blue} Docker mode activated. {color} |
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m
1s{color} | {color:green} No case conflicting files found. {color} |
| {color:blue}0{color} | {color:blue} shelldocs {color} | {color:blue} 0m
0s{color} | {color:blue} Shelldocs was not available. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m
0s{color} | {color:green} The patch appears to include 15 new or modified test
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 1m
13s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 23m
10s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 19m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 3m
6s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 5m
51s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 46s{color} | {color:green} branch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 4m
37s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue} 5m
20s{color} | {color:blue} Used deprecated FindBugs config; considering
switching to SpotBugs. {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-project no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
31s{color} | {color:blue} branch/hadoop-assemblies no findbugs output file
(findbugsXml.xml) {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
32s{color} | {color:blue} branch/hadoop-tools/hadoop-tools-dist no findbugs
output file (findbugsXml.xml) {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
30s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 4m
46s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 17m
26s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 17m
26s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
2m 53s{color} | {color:orange} root: The patch generated 19 new + 2 unchanged -
0 fixed = 21 total (was 2) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 6m
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} shellcheck {color} | {color:green} 0m
0s{color} | {color:green} There were no new shellcheck issues. {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m
7s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green}
15m 37s{color} | {color:green} patch has no errors when building and testing
our client artifacts. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 5m
15s{color} | {color:green} the patch passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
31s{color} | {color:blue} hadoop-project has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0m
33s{color} | {color:blue} hadoop-assemblies has no data from findbugs {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 0
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17121517#comment-17121517
]
Jinglun commented on HDFS-15346:
Hi [~linyiqun], thanks your great comments ! Yes it is a big patch, very
thankful for reviewing this ! Shoot all the suggestions and upload v05.
{quote}line 132: Why the default bandwidth is only 1 for fedbaalance, will not
be too small?
{quote}
Yes it's too small. I make the default to 10.
{quote}We don't need to use nnUri here because we have already got the
Filesystem instance. If we don't want to specified for one namespace, URI
prefix can be ignored, default fs will be used.
{quote}
I was using the nnUri because when the TrashProcedure is ran in
DistCpFedBalance, the source path and dst path are full paths with the cluster
information. Simplify to use the default fs is also ok as using the URI prefix
is not intuitive. I change it to the simple way.
> 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
>
>
> 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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-15346) RBF: DistCpFedBalance implementation
[
https://issues.apache.org/jira/browse/HDFS-15346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17120981#comment-17120981
]
Yiqun Lin commented on HDFS-15346:
--
Hi [~LiJinglun] , some initial review comments from me:
*DistCpFedBalance.java*
# line 77 I suggest to extract 'submit' as a static variable in this class.
# line 85 the same comment to extract.
# line 127 Can you complete the javadoc of this method?
# line 132: Why the default bandwidth is only 1 for fedbaalance, will not be
too small?
# line 137, 140, 150 We can use method CommandLine#hasOption to extract
Boolean type input value.
# line 178 Can you complete the javadoc of construct method?
# line 199, 206, 210, 215 Also suggest to use static variable rather than
hard-coded value in these places.
# line 228 rClient not closed after it's used.
*DistCpProcedure.java*
# line 191 We can use HdfsConstants.SEPARATOR_DOT_SNAPSHOT_DIR_SEPARATOR to
replace '/.snapshot/'
# line 306 It will be better if we can add some necessary describe for the
steps of diff distcp job submission.
# line 374 Can we replace '.snapshot' with HdfsConstants.DOT_SNAPSHOT_DIR in
all other places in this class?
*TestDistCpProcedure.java*
Can you use replace HdfsConstants.DOT_SNAPSHOT_DIR to replace '.snapshot' in
this class?
*TestTrashProcedure.java*
{quote}Path src = new Path(nnUri + "/"+getMethodName()+"-src");
Path dst = new Path(nnUri + "/"+getMethodName()+"-dst");
{quote}
We don't need to use nnUri here because we have already got the Filesystem
instance. If we don't want to specified for one namespace, URI prefix can be
ignored, default fs will be used.
We can simplifed to
{quote}Path src = new Path("/"+getMethodName()+"-src");
Path dst = new Path("/"+getMethodName()+"-dst");
{quote}
> 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
>
>
> 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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
