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

Yiqun Lin commented on HDFS-15340:
----------------------------------

Additional comments for the unit test:
{code:java}
+    CONF.setBoolean("dfs.namenode.acls.enabled", true);
{code}
Use DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY instead of.

Method *testShutdownScheduler*
{code:java}
    builder.nextProcedure(new WaitProcedure("wait", 1000, 30 * 1000));
+    BalanceJob job = builder.build();
+
+    scheduler.submit(job);
+    Thread.sleep(1000); // wait job to be scheduled.
+    scheduler.shutDownAndWait(30 * 1000);
{code}
For the testing, we don't have to use 30s to wait. It's so long time, 5s is 
same.
  
 Method *testGetJobAfterRecover*
{code:java}
+      assertNull(recoverJob.error());
+      assertNotSame(job, recoverJob);
+      assertEquals(job, recoverJob)
{code}
We need verify more detail info of recovered job, like firstProcedure, 
curProcedure and lastProcedure in this BalanceJob. This can completely check 
that the job is correctly recovered.
 So we should be make adjustment in {{BalanceJob#equal}}
{code:java}
  @Override
  public boolean equals(Object obj) {
    if (obj == null) {
      return false;
    }
    if (obj == this) {
      return true;
    }
    if (obj.getClass() != getClass()) {
      return false;
    }
    BalanceJob bj = (BalanceJob) obj;
    return new EqualsBuilder()
        .append(id, bj.id)
        .append(procedureTable, bj.procedureTable)
        .append(jobDone, bj.jobDone)
        .append(firstProcedure, bj.firstProcedure)
        .append(curProcedure, bj.curProcedure)
        .append(lastProcedure, bj.lastProcedure)
        .isEquals();
  }
{code}
Method *testSchedulerDownAndRecoverJob*
  
{code:java}
int len = fs.listStatus(parent).length;
+      assertTrue(len > 0 && len < 10);
+      // restart scheduler, test recovering the job.
+      scheduler = new BalanceProcedureScheduler(CONF);
+      scheduler.init(true);
+      scheduler.waitUntilDone(job);
{code}
Can we also verify the recovered job here?

Method *testRetry*
 Use Time.monotonicNow() to replace Time.now().

Additional two minor comments for method naming in BalanceJob:
 * public boolean removeAfterDone() --> shouldRemoveAfterDone()
 * public Exception error() --> gerError()

 

> RBF: Implement BalanceProcedureScheduler basic framework
> --------------------------------------------------------
>
>                 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

Reply via email to