Review Board 1.7.22


HBASE-5081 Distributed log splitting deleteNode races againsth splitLog retry

Review Request #3292 - Created Dec. 21, 2011 and discarded

Jimmy Xiang
0.92
HBASE-5081
Reviewers
hbase
lhofhansl, stack, tedyu
hbase-git
In this patch, after a task is done, we don't delete the node if the task is failed.  So that when it's retried later on, there won't be race problem.

It used to delete the node always.
mvn -Dtest=TestDistributedLogSplitting clean test

Diff revision 8 (Latest)

1 2 3 4 5 6 7 8
1 2 3 4 5 6 7 8

  1. src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java: Loading...
  2. src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java: Loading...
src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
Revision 667a8b1 New Change
[20] 329 lines
[+20] [+] private void setDone(String path, TerminationStatus status) {
330
      } else {
330
      } else {
331
        tot_mgr_log_split_err.incrementAndGet();
331
        tot_mgr_log_split_err.incrementAndGet();
332
        LOG.warn("Error splitting " + path);
332
        LOG.warn("Error splitting " + path);
333
      }
333
      }
334
    }
334
    }

    
   
335
    boolean safeToDeleteNodeAsync = true;
335
    Task task = tasks.get(path);
336
    Task task = tasks.get(path);
336
    if (task == null) {
337
    if (task == null) {
337
      if (!ZKSplitLog.isRescanNode(watcher, path)) {
338
      if (!ZKSplitLog.isRescanNode(watcher, path)) {
338
        tot_mgr_unacquired_orphan_done.incrementAndGet();
339
        tot_mgr_unacquired_orphan_done.incrementAndGet();
339
        LOG.debug("unacquired orphan task is done " + path);
340
        LOG.debug("unacquired orphan task is done " + path);
340
      }
341
      }
341
    } else {
342
    } else {
342
      synchronized (task) {
343
      synchronized (task) {
343
        task.deleted = true;
344
        task.deleted = true;
344
        // if in stopTrackingTasks() we were to make tasks orphan instead of
345
        // if in stopTrackingTasks() we were to make tasks orphan instead of
345
        // forgetting about them then we will have to handle the race when
346
        // forgetting about them then we will have to handle the race when
346
        // accessing task.batch here.
347
        // accessing task.batch here.
347
        if (!task.isOrphan()) {
348
        if (!task.isOrphan()) {

    
   
349
          if (status != SUCCESS) {

    
   
350
            // If the task is failed, deleting the node asynchronously

    
   
351
            // will cause race issue against split log retry.

    
   
352
            // In this case, we should delete it now.

    
   
353
            safeToDeleteNodeAsync = false;

    
   
354
            deleteNodeNow(path);

    
   
355
          }
348
          synchronized (task.batch) {
356
          synchronized (task.batch) {
349
            if (status == SUCCESS) {
357
            if (status == SUCCESS) {
350
              task.batch.done++;
358
              task.batch.done++;
351
            } else {
359
            } else {
352
              task.batch.error++;
360
              task.batch.error++;
[+20] [20] 4 lines
[+20] private void setDone(String path, TerminationStatus status) {
357
      }
365
      }
358
    }
366
    }
359
    // delete the task node in zk. Keep trying indefinitely - its an async
367
    // delete the task node in zk. Keep trying indefinitely - its an async
360
    // call and no one is blocked waiting for this node to be deleted. All
368
    // call and no one is blocked waiting for this node to be deleted. All
361
    // task names are unique (log.<timestamp>) there is no risk of deleting
369
    // task names are unique (log.<timestamp>) there is no risk of deleting
362
    // a future task.
370
    // a future task.  This is true if the task status is SUCCESS, otherwise,

    
   
371
    // it may race against split log retry.

    
   
372
    if (safeToDeleteNodeAsync) {
363
    deleteNode(path, Long.MAX_VALUE);
373
      deleteNode(path, Long.MAX_VALUE);

    
   
374
    }
364
    return;
375
    return;
365
  }
376
  }
366

    
   
377

   

    
   
378
  private void deleteNodeNow(String path) {

    
   
379
    try {

    
   
380
      tot_mgr_node_delete_queued.incrementAndGet();

    
   
381
      this.watcher.getRecoverableZooKeeper().delete(path, -1);

    
   
382
      tot_mgr_task_deleted.incrementAndGet();

    
   
383
    } catch (KeeperException ke) {

    
   
384
      if (ke.code() != KeeperException.Code.NONODE) {

    
   
385
        tot_mgr_node_delete_err.incrementAndGet();

    
   
386
        LOG.warn("Failed to delete failed task node: "

    
   
387
          + path + " due to " + ke.getMessage());

    
   
388
      } else {

    
   
389
        LOG.info("Failed task node does not exist, "

    
   
390
            + "either was never created or was already deleted: " + path);

    
   
391
        tot_mgr_task_deleted.incrementAndGet();

    
   
392
      }

    
   
393
    } catch (InterruptedException ie) {

    
   
394
      LOG.warn("Interrupted while waiting for failed task node to be deleted");

    
   
395
      Thread.currentThread().interrupt();

    
   
396
    }

    
   
397
  }

    
   
398

   
367
  private void createNode(String path, Long retry_count) {
399
  private void createNode(String path, Long retry_count) {
368
    ZKUtil.asyncCreate(this.watcher, path,
400
    ZKUtil.asyncCreate(this.watcher, path,
369
        TaskState.TASK_UNASSIGNED.get(serverName), new CreateAsyncCallback(),
401
        TaskState.TASK_UNASSIGNED.get(serverName), new CreateAsyncCallback(),
370
        retry_count);
402
        retry_count);
371
    tot_mgr_node_create_queued.incrementAndGet();
403
    tot_mgr_node_create_queued.incrementAndGet();
[+20] [20] 720 lines
src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java
Revision 32ad7e8 New Change
 
  1. src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java: Loading...
  2. src/test/java/org/apache/hadoop/hbase/master/TestSplitLogManager.java: Loading...