[jira] [Commented] (HDFS-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-12 Thread Yiqun Lin (Jira)


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

Yiqun Lin commented on HDFS-15340:
--

Thanks for addressing the comments, [~LiJinglun].

Some places are confused to understand, also you need to take care for some 
typos, :).

Some more review comments:

*BalanceJob.java*
 Typo: {{Each procedure need}} --> {{Each procedure needs}}
 Typo: jos -> job

Can you rewrite this:
{noformat}
Start procedure {}. The last procedure is ... -- > Start procedure {}, last 
procedure is 
{noformat}
How about adding sleep interval time when curProcedure.execute returning false 
that means curProcedure executed failed? It can avoid frequently executing 
failed procedure.
{noformat}
if (curProcedure.execute(lastProcedure)) {
+lastProcedure = curProcedure;
+curProcedure = next();
+  }
{noformat}
The balancer job is same, why we always write its journal info to HDFS? Only 
once time is enough I think.
{noformat}
if (!scheduler.writeJournal(this)) {
+quit = true; // Write journal failed. Simply quit because this job
+ // has already been added to the recoverQueue.
+LOG.debug("Write journal failed. Quit and wait for recovery.");
+  }
{noformat}
I see this is also used for testing, can you add @VisibleForTesting annotation 
for this?
{noformat}
+  public boolean removeAfterDone() {
+return removeAfterDone;
+  }{noformat}
It would be better to add comment for this method, we don't know why we pass 
exception here.
{code:java}
private synchronized void finish(Exception exception)
{code}
*BalanceJournalInfoHDFS.java*
 Found many minor grammar rule issues, can you have a quick fix?
 * "list all jobs from journal"; – > "List all jobs from journal"
 * Need to leave one white space: builder.append(",") -> builder.append(", ");
 * clear journal of job - > Clear journal of job

*BalanceProcedure.java*
{code:java}
+  /**
+   * The main process. This is called by the ProcedureScheduler.
+
+   * Make sure the process quits fast when it's interrupted and the scheduler 
is
+   * shut down.
+   *
+   * @param lastProcedure the last procedure.
+   * @throws RetryException if this procedure needs delay a while then retry.
+   * @return true if the procedure has done and the job will go to the next
+   * procedure, otherwise false.
+   */
+  public abstract boolean execute(T lastProcedure)
+  throws RetryException, IOException;
{code}
lastProcedure here is only used for testing, I suggest to remove this as an 
input parameter. It seems too confused that we pass lastProcedure but do 
nothing in actual BalanceProcedure class. The major function methods need be 
clear for others to understand, :).

*BalanceProcedureScheduler.java*
 For the elapse time calculation on Hadoop world, we will use 
Time.monotonicNow() not Time.now or System.currentTimeMillis(). Can you update 
for this?
{noformat}
this.time = Time.now() + delayInMilliseconds;
long delay = time - System.currentTimeMillis();
{noformat}
*UnrecoverableProcedure.java*
{code:java}
+  @Override
+  public boolean execute(BalanceProcedure lastProcedure) throws RetryException,
+  IOException {
+if (handler != null) {
+  return handler.execute(lastProcedure);
+} else {
+  return true;
+}
+  }
{code}
We could use mock to throw exception, not depend on BalanceProcedure passed to 
throw exception. So lastProcedure can completely removed in execute method.

> RBF: Balance data across federation namespaces with DistCp and snapshot diff 
> / Step 1: The State Machine(BalanceProcedureScheduler)
> ---
>
> Key: HDFS-15340
> URL: https://issues.apache.org/jira/browse/HDFS-15340
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Jinglun
>Assignee: Jinglun
>Priority: Major
> Attachments: HDFS-15340.001.patch, HDFS-15340.002.patch, 
> HDFS-15340.003.patch, HDFS-15340.004.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This 
> is the first 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-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-11 Thread Hadoop QA (Jira)


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

Hadoop QA commented on HDFS-15340:
--

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
40s{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 6 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 18m 
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
14m 59s{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}  0m 
34s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue}  1m 
13s{color} | {color:blue} Used deprecated FindBugs config; considering 
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
11s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
32s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
28s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
28s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
32s{color} | {color:green} the patch passed {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} shadedclient {color} | {color:green} 
13m 46s{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}  0m 
32s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
13s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} unit {color} | {color:red}  8m  6s{color} 
| {color:red} hadoop-hdfs-rbf in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
31s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 65m 17s{color} | 
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | 
hadoop.hdfs.server.federation.router.TestRouterRpcMultiDestination |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: 
https://builds.apache.org/job/PreCommit-HDFS-Build/29264/artifact/out/Dockerfile
 |
| JIRA Issue | HDFS-15340 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/13002587/HDFS-15340.004.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite 
unit shadedclient findbugs checkstyle |
| uname | Linux ddb8f9e31add 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 
11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / aab9e0b16ec |
| Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| unit | 
https://builds.apache.org/job/PreCommit-HDFS-Build/29264/artifact/out/patch-unit-hadoop-hdfs-project_hadoop-hdfs-rbf.txt
 |
|  Test Results | 

[jira] [Commented] (HDFS-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-10 Thread Jinglun (Jira)


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

Jinglun commented on HDFS-15340:


Upload v04, fix checkstyle.

> RBF: Balance data across federation namespaces with DistCp and snapshot diff 
> / Step 1: The State Machine(BalanceProcedureScheduler)
> ---
>
> Key: HDFS-15340
> URL: https://issues.apache.org/jira/browse/HDFS-15340
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Jinglun
>Assignee: Jinglun
>Priority: Major
> Attachments: HDFS-15340.001.patch, HDFS-15340.002.patch, 
> HDFS-15340.003.patch, HDFS-15340.004.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This 
> is the first 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-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-10 Thread Hadoop QA (Jira)


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

Hadoop QA commented on HDFS-15340:
--

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  1m 
55s{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 6 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 20m 
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
35s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
14m 38s{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}  0m 
33s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue}  1m 
11s{color} | {color:blue} Used deprecated FindBugs config; considering 
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m  
9s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
29s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
29s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}  
0m 17s{color} | {color:orange} hadoop-hdfs-project/hadoop-hdfs-rbf: The patch 
generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
31s{color} | {color:green} the patch passed {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} shadedclient {color} | {color:green} 
13m 42s{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}  0m 
30s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
12s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  8m 
10s{color} | {color:green} hadoop-hdfs-rbf in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
32s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 67m 47s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: 
https://builds.apache.org/job/PreCommit-HDFS-Build/29263/artifact/out/Dockerfile
 |
| JIRA Issue | HDFS-15340 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/13002576/HDFS-15340.003.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite 
unit shadedclient findbugs checkstyle |
| uname | Linux 09cbeea788d0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 
11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / aab9e0b16ec |
| Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
| checkstyle | 
https://builds.apache.org/job/PreCommit-HDFS-Build/29263/artifact/out/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs-rbf.txt
 |
|  Test Results | 

[jira] [Commented] (HDFS-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-10 Thread Jinglun (Jira)


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

Jinglun commented on HDFS-15340:


Hi [~ayushtkn] [~linyiqun], thanks your nice review, comments and suggestions ! 
Upload v03.

 
{quote}BalanceJob.java and BalanceProcedureConfigKeys.java both have private 
empty constructors, give a check if you would need them?
{quote}
The empty constructor of BalanceJob is useless, I'll remove it.   The private 
empty constructor in BalanceProcedureConfigKeys.java is because of checkstyle.  
Something like: tool class must not have public constructor. I'll keep it to 
pass checkstyle.

 
{quote}Don't need I think and I don't find the place that will invoke following 
construct method.
{quote}
The empty constructor for BalanceProcedure, MultiPhaseProcedure, 
RecordProcedure and RetryProcedure a
re used for deserialization. It uses reflection and needs a standard empty 
constructor. The deserialization code is at BalanceJob.readFields().

 

A small reminder is the HDFSJournal is renamed to BalanceJournalInfoHDFS in v03.

> RBF: Balance data across federation namespaces with DistCp and snapshot diff 
> / Step 1: The State Machine(BalanceProcedureScheduler)
> ---
>
> Key: HDFS-15340
> URL: https://issues.apache.org/jira/browse/HDFS-15340
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Jinglun
>Assignee: Jinglun
>Priority: Major
> Attachments: HDFS-15340.001.patch, HDFS-15340.002.patch, 
> HDFS-15340.003.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This 
> is the first 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-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-10 Thread Yiqun Lin (Jira)


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

Yiqun Lin commented on HDFS-15340:
--

Haven't taken the deep review, but some initial review comments for the 
readable of this patch:

*BalanceJob.java*
{noformat}
+ public static final Logger LOG =
+ LoggerFactory.getLogger(BalanceJob.class.getName());
{noformat}
{{BalanceJob.class.getName()}} can be simplified to {{BalanceJob.class}}
{noformat}
private BalanceJob() {}
{noformat}
I don't think this is necessary since we already define {{private 
BalanceJob(Iterable procedures, boolean remove)}}.

In BalanceJob#toString, can we add missed comma character in string builder 
when doing the new append operation?

*BalanceProcedure.java*
{noformat}
public static final Logger LOG =
 + LoggerFactory.getLogger(BalanceProcedure.class.getName());
{noformat}
BalanceProcedure.class.getName() --> BalanceProcedure.class

It will look better if we could add some necessary comments for these variables.
{noformat}
+  private String nextProcedure;
+  private String name;
+  private long delayDuration;
+  private BalanceJob job;

{noformat}
Don't need I think and I don't find he place that will invoke following 
construct method.
{noformat}
public BalanceProcedure() {  }
{noformat}
*BalanceProcedureScheduler.java*
{noformat}
+  private ConcurrentHashMap jobSet;
+  private LinkedBlockingQueue runningQueue;
+  private DelayQueue delayQueue;
+  private LinkedBlockingQueue recoverQueue;
+  private Configuration conf;
+  private BalanceJournal journal;
+
+  private Thread reader;
+  private ThreadPoolExecutor workersPool;
+  private Thread rooster;
+  private Thread recoverThread;
+  private AtomicBoolean running = new AtomicBoolean(true);
{noformat}
Two suggestions for above lines:
 * Can we add some necessary comments for these variables?
 * Can we unified the naming pattern of thread names, like reader -> 
readerThread, rooster -> roosterThread?

*HDFSJournal.java*
{noformat}
+/**
+ * Journal based on HDFS.
+ */
+public class HDFSJournal implements BalanceJournal {
{noformat}
The class HDFSJournal looks a little confused, can we rename this to a more 
readable name like BalanceJournalInfo?

In addition, please add more description for this java doc description.

Can we print some tracking log in saveJob/recoverJob/listAllJobs/clear methods? 
It will let users known detailed operation of Balancer job journal operations.

*MultiPhaseProcedure.java*
 * LOG.info("phase {}", currentPhase); --> LOG.info("Current phase {}", 
currentPhase);
 * Not needed: {{public MultiPhaseProcedure() {}}}

*RecordProcedure.java*
 Not needed: {{public RecordProcedure() {}}}

*RetryProcedure.java*
 Not needed: {{public RetryProcedure() {}}}

I will give my further detailed review comments soon.

> RBF: Balance data across federation namespaces with DistCp and snapshot diff 
> / Step 1: The State Machine(BalanceProcedureScheduler)
> ---
>
> Key: HDFS-15340
> URL: https://issues.apache.org/jira/browse/HDFS-15340
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Jinglun
>Assignee: Jinglun
>Priority: Major
> Attachments: HDFS-15340.001.patch, HDFS-15340.002.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This 
> is the first 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-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-09 Thread Ayush Saxena (Jira)


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

Ayush Saxena commented on HDFS-15340:
-


In {{HDFSJournal.java}}

In saveJob(BalanceJob job) 

 can use directly {{new Path(jobFile + TMP_TAIL);}} for tmpJobFile


{code:java}
  if (out != null) {
try {
  out.close();
} catch (IOException e) {
}
  }

{code}
Exceptions shouldn't be silently ignored, better use IOUtils.closeStream(out)

In {{recoverJob(BalanceJob job)}}
assert job.getId() != null;
If this fails, this would surface, it won't be caught in 
{{BalancerProcedureScheduler#run}} and the job won't be removed. Not pretty 
sure when this exception will trigger. Give a check if it can be smoothlly 
handled.

In {{listAllJobs()}} :

Can add a debug log for FNF exception.

{{new Path(workUri.getPath())}} this is used multiple times, can refactor as 
single variable.

We are using listStatus and getFileStatus both on the same path, getFileStatus 
to check directory exists, can we use only listStatus. listStatus also returns 
FNF if directory isn't there. Can save one rpc call?

In {{BalanceJob.java}}
 In {{nextProcedure()}}
Either reuse {{procedures.size()}} by refactoring to variable else check 
{{isEmpty()}}

In {{execute}} method :
  {{while (!jobDone && scheduler.isRunning() && !quit)}} : quit is a local 
variable, this can be checked before {{scheduler.isRunning}}

For  quit = true; better to add a debug log too here, if in future things don't 
work, this may help then. Better to know it got exited abnormally.

In BalanceProcedure.java we can have a toString() too returning the name, so we 
can directly use the procedure in Logs, StringBuilders and relevant places

BalanceJob.java and BalanceProcedureConfigKeys.java both have private empty 
constructors, give a check if you would need them?

In BalanceProcedureScheduler.java 

  final BalanceJob job = runningQueue.take();
In case of any code/enviornment issue later, this take can get stuck 
infinitely, as a preventive measure I think we should have a timeout here for 
safety. {{DFSStripedOutputStream}} has a example {{takeWithTimeout}} 
implemented, Can give a check. Similarly in {{Recover#run}}

 {{LOG.info("Start job. job={}", job);}}
This won't work if job doesn't have a toString implemented, either use 
.name/.id or implement toString in BalanceJob. Give a check to the logs below 
too they are using ID, either use ID for all or name or both everywhere in logs.

Haven't checked the tests line by line, but they doesn't seams flaky atleast, 
in multiple runs. All are working fine.

[~elgoiri] can you too have  a look once.

> RBF: Balance data across federation namespaces with DistCp and snapshot diff 
> / Step 1: The State Machine(BalanceProcedureScheduler)
> ---
>
> Key: HDFS-15340
> URL: https://issues.apache.org/jira/browse/HDFS-15340
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Jinglun
>Assignee: Jinglun
>Priority: Major
> Attachments: HDFS-15340.001.patch, HDFS-15340.002.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This 
> is the first 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-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-09 Thread Hadoop QA (Jira)


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

Hadoop QA commented on HDFS-15340:
--

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  0m 
51s{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 6 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 19m 
 4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
36s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
27s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
15m  5s{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}  0m 
34s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue}  1m  
8s{color} | {color:blue} Used deprecated FindBugs config; considering switching 
to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m  
7s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
33s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
28s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
28s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
17s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
30s{color} | {color:green} the patch passed {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} shadedclient {color} | {color:green} 
13m 44s{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}  0m 
30s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
14s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  8m  
3s{color} | {color:green} hadoop-hdfs-rbf in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
33s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 65m 32s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: 
https://builds.apache.org/job/PreCommit-HDFS-Build/29258/artifact/out/Dockerfile
 |
| JIRA Issue | HDFS-15340 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/13002490/HDFS-15340.002.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite 
unit shadedclient findbugs checkstyle |
| uname | Linux 13c3c2d8d828 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 
11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / c784ba370ee |
| Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
|  Test Results | 
https://builds.apache.org/job/PreCommit-HDFS-Build/29258/testReport/ |
| Max. process+thread count | 3501 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: 
hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output | 

[jira] [Commented] (HDFS-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-09 Thread Jinglun (Jira)


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

Jinglun commented on HDFS-15340:


Upload v02, using HashCodeBuilder and EqualsBuilder for hashCode() and equals().

> RBF: Balance data across federation namespaces with DistCp and snapshot diff 
> / Step 1: The State Machine(BalanceProcedureScheduler)
> ---
>
> Key: HDFS-15340
> URL: https://issues.apache.org/jira/browse/HDFS-15340
> Project: Hadoop HDFS
>  Issue Type: Sub-task
>Reporter: Jinglun
>Assignee: Jinglun
>Priority: Major
> Attachments: HDFS-15340.001.patch, HDFS-15340.002.patch
>
>
> Patch in HDFS-15294 is too big to review so we split it into 2 patches. This 
> is the first 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-15340) RBF: Balance data across federation namespaces with DistCp and snapshot diff / Step 1: The State Machine(BalanceProcedureScheduler)

2020-05-06 Thread Hadoop QA (Jira)


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

Hadoop QA commented on HDFS-15340:
--

| (/) *{color:green}+1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue}  1m 
57s{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 6 new or modified test 
files. {color} |
|| || || || {color:brown} trunk Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 23m 
17s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
32s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
22s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
37s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 
17m 43s{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}  0m 
33s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} spotbugs {color} | {color:blue}  1m 
27s{color} | {color:blue} Used deprecated FindBugs config; considering 
switching to SpotBugs. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
24s{color} | {color:green} trunk passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
35s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
32s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
32s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green}  0m 
36s{color} | {color:green} the patch passed {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} shadedclient {color} | {color:green} 
16m 21s{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}  0m 
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  1m 
29s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:green}+1{color} | {color:green} unit {color} | {color:green}  9m  
6s{color} | {color:green} hadoop-hdfs-rbf in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green}  0m 
29s{color} | {color:green} The patch does not generate ASF License warnings. 
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 77m 47s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | ClientAPI=1.40 ServerAPI=1.40 base: 
https://builds.apache.org/job/PreCommit-HDFS-Build/29244/artifact/out/Dockerfile
 |
| JIRA Issue | HDFS-15340 |
| JIRA Patch URL | 
https://issues.apache.org/jira/secure/attachment/13002247/HDFS-15340.001.patch |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite 
unit shadedclient findbugs checkstyle |
| uname | Linux 1f60015af5ac 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 
08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | personality/hadoop.sh |
| git revision | trunk / 99840aaba66 |
| Default Java | Private Build-1.8.0_252-8u252-b09-1~18.04-b09 |
|  Test Results | 
https://builds.apache.org/job/PreCommit-HDFS-Build/29244/testReport/ |
| Max. process+thread count | 3794 (vs. ulimit of 5500) |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-rbf U: 
hadoop-hdfs-project/hadoop-hdfs-rbf |
| Console output |