Hadoop Common
  1. Hadoop Common
  2. HADOOP-4935

Manual leaving of safe mode may lead to data lost

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.18.3
    • Fix Version/s: 0.18.3
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Due to HADOOP-4610, NameNode calculates mis-replicated blocks when leaving safe mode manually, where it clears the pending deletion queue before it does the calculation. This works fine when NameNode just starts but introduced a bug when NameNode is running for a while. Clearing the pending deletion queue makes NameNode not able to distinguish valid replicas from invalid ones, ie, the ones that have scheduled or dispatched for deletion. Therefore, NameNode may mistakenly decide the block is over-replicated and choose all valid ones to delete.

      1. misReplBlocks-0-18.patch
        0.6 kB
        Konstantin Shvachko
      2. misReplBlocks.patch
        0.7 kB
        Konstantin Shvachko

        Activity

        Hide
        Konstantin Shvachko added a comment -

        By "pending deletion queue" you mean excessReplicateMap I believe.
        This happens when data-nodes are flaky or busy and name-node looses them and replicates blocks to other nodes, but the old data-nodes come back and report extra (old) replicas for blocks that have already been re-replicated.
        Thus the block becomes over-replicated and therefore is placed into excessReplicateMap.
        The data-nodes delete excessive replicas, but before the deletion is reported back to the name-node the latter calls processMisReplicatedBlocks() which clears excessReplicateMap in an attempt to restore it from scratch.
        The error is that after clearing excessReplicateMap all replicas become valid from the name-node point of view although some of them have already been deleted by data-nodes. So if there were 6 replicas of the same block and before processMisReplicatedBlocks() first three were deleted, then after processMisReplicatedBlocks() clears excessReplicateMap the other three can be also removed and there will be no more replicas of the block.

        Show
        Konstantin Shvachko added a comment - By "pending deletion queue" you mean excessReplicateMap I believe. This happens when data-nodes are flaky or busy and name-node looses them and replicates blocks to other nodes, but the old data-nodes come back and report extra (old) replicas for blocks that have already been re-replicated. Thus the block becomes over-replicated and therefore is placed into excessReplicateMap . The data-nodes delete excessive replicas, but before the deletion is reported back to the name-node the latter calls processMisReplicatedBlocks() which clears excessReplicateMap in an attempt to restore it from scratch. The error is that after clearing excessReplicateMap all replicas become valid from the name-node point of view although some of them have already been deleted by data-nodes. So if there were 6 replicas of the same block and before processMisReplicatedBlocks() first three were deleted, then after processMisReplicatedBlocks() clears excessReplicateMap the other three can be also removed and there will be no more replicas of the block.
        Hide
        Konstantin Shvachko added a comment -

        This fixes the problem.
        I tried to make a unit test, but it is really hard to make it deterministic.

        Show
        Konstantin Shvachko added a comment - This fixes the problem. I tried to make a unit test, but it is really hard to make it deterministic.
        Hide
        Konstantin Shvachko added a comment -

        This is the patch for 0.18 branch.
        I tested it manually. I reproduced the error with current code and verified this is not happening with the patch.

        Show
        Konstantin Shvachko added a comment - This is the patch for 0.18 branch. I tested it manually. I reproduced the error with current code and verified this is not happening with the patch.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        ant test-patch results:

             [exec] -1 overall.  
             [exec] 
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec] 
             [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
             [exec]                         Please justify why no tests are needed for this patch.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
        
        Show
        Tsz Wo Nicholas Sze added a comment - ant test-patch results: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
        Hide
        Suresh Srinivas added a comment -

        +1 patch looks good

        Show
        Suresh Srinivas added a comment - +1 patch looks good
        Hide
        Konstantin Shvachko added a comment -

        All tests except TestMapReduceLocal passed.

        Show
        Konstantin Shvachko added a comment - All tests except TestMapReduceLocal passed.
        Hide
        Konstantin Shvachko added a comment -

        I just committed this.

        Show
        Konstantin Shvachko added a comment - I just committed this.

          People

          • Assignee:
            Konstantin Shvachko
            Reporter:
            Hairong Kuang
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development