Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0
    • Fix Version/s: 3.0.0, 2.3.0
    • Component/s: datanode
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Target Version/s:

      Description

      The DataNode's UUID (DataStorage.getDatanodeUuid() field) is NULL at the point where the FsDataset object is created (DataNode.initStorage().

      As the DataStorage object is an input to the FsDataset factory method, it is desirable for it to be fully populated with a UUID at this point. In particular, our FsDatasetSpi implementation relies upon the DataNode UUID as a key to access our underlying block storage device.

      This also appears to be a regression compared to Hadoop 1.x - our 1.x FSDatasetInterface plugin has a non-NULL UUID on startup. I haven't fully traced through the code, but I suspect this came from the BPOfferService/BPServiceActor refactoring to support federated namenodes.

      With HDFS-5448, the DataNode is now responsible for generating its own UUID. This greatly simplifies the fix. Move the UUID check/generation in from DataNode.createBPRegistration() to DataNode.initStorage(). This more naturally co-locates UUID generation immediately subsequent to the read of the UUID from the DataStorage properties file.

        private void initStorage(final NamespaceInfo nsInfo) throws IOException {
          // ...
      
            final String bpid = nsInfo.getBlockPoolID();
            //read storage info, lock data dirs and transition fs state if necessary
            storage.recoverTransitionRead(this, bpid, nsInfo, dataDirs, startOpt);
            
            // SUGGESTED NEW PLACE TO CHECK DATANODE UUID
            checkDatanodeUuid();
      
          // ...
        }
      
      1. HDFS-5454.01.patch
        1 kB
        Arpit Agarwal
      2. HDFS-5454.02.patch
        1 kB
        Arpit Agarwal
      3. HDFS-5454.03.patch
        5 kB
        Arpit Agarwal

        Activity

        Hide
        Arpit Agarwal added a comment -

        Merged to branch-2. Target version set to 2.4.0.

        Show
        Arpit Agarwal added a comment - Merged to branch-2. Target version set to 2.4.0.
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Mapreduce-trunk #1641 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1641/)
        HDFS-5454. DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1641 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1641/ ) HDFS-5454 . DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-Hdfs-trunk #1615 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1615/)
        HDFS-5454. DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1615 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1615/ ) HDFS-5454 . DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Hide
        Hudson added a comment -

        FAILURE: Integrated in Hadoop-Yarn-trunk #424 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/424/)
        HDFS-5454. DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Show
        Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #424 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/424/ ) HDFS-5454 . DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Hadoop-trunk-Commit #4889 (See https://builds.apache.org/job/Hadoop-trunk-Commit/4889/)
        HDFS-5454. DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296)

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Show
        Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #4889 (See https://builds.apache.org/job/Hadoop-trunk-Commit/4889/ ) HDFS-5454 . DataNode UUID should be assigned prior to FsDataset initialization. (Arpit Agarwal) (arp: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1551296 ) /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataNodeInitStorage.java
        Hide
        Arpit Agarwal added a comment -

        Thanks Junping (and congrats on your committer +1). I committed it to trunk.

        Show
        Arpit Agarwal added a comment - Thanks Junping (and congrats on your committer +1). I committed it to trunk.
        Hide
        Junping Du added a comment -

        +1. Patch looks good to me. Thanks for this refactoring. Aript!

        Show
        Junping Du added a comment - +1. Patch looks good to me. Thanks for this refactoring. Aript!
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12618793/HDFS-5454.03.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5723//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5723//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12618793/HDFS-5454.03.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5723//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5723//console This message is automatically generated.
        Hide
        Arpit Agarwal added a comment -

        Update patch to add test case.

        Show
        Arpit Agarwal added a comment - Update patch to add test case.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12618671/HDFS-5454.02.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5713//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5713//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12618671/HDFS-5454.02.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5713//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5713//console This message is automatically generated.
        Hide
        Arpit Agarwal added a comment -

        Updated patch.

        Show
        Arpit Agarwal added a comment - Updated patch.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12618477/HDFS-5454.01.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.TestInjectionForSimulatedStorage
        org.apache.hadoop.hdfs.TestPread
        org.apache.hadoop.hdfs.TestReplication
        org.apache.hadoop.hdfs.TestSmallBlock
        org.apache.hadoop.hdfs.server.balancer.TestBalancerWithNodeGroup
        org.apache.hadoop.hdfs.server.datanode.TestDataNodeMetrics
        org.apache.hadoop.hdfs.server.balancer.TestBalancerWithEncryptedTransfer
        org.apache.hadoop.hdfs.TestFileCreation
        org.apache.hadoop.hdfs.TestSetrepIncreasing
        org.apache.hadoop.hdfs.server.balancer.TestBalancerWithHANameNodes
        org.apache.hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
        org.apache.hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFS
        org.apache.hadoop.hdfs.server.namenode.TestFileLimit
        org.apache.hadoop.hdfs.server.balancer.TestBalancer

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5708//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5708//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12618477/HDFS-5454.01.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.TestInjectionForSimulatedStorage org.apache.hadoop.hdfs.TestPread org.apache.hadoop.hdfs.TestReplication org.apache.hadoop.hdfs.TestSmallBlock org.apache.hadoop.hdfs.server.balancer.TestBalancerWithNodeGroup org.apache.hadoop.hdfs.server.datanode.TestDataNodeMetrics org.apache.hadoop.hdfs.server.balancer.TestBalancerWithEncryptedTransfer org.apache.hadoop.hdfs.TestFileCreation org.apache.hadoop.hdfs.TestSetrepIncreasing org.apache.hadoop.hdfs.server.balancer.TestBalancerWithHANameNodes org.apache.hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes org.apache.hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFS org.apache.hadoop.hdfs.server.namenode.TestFileLimit org.apache.hadoop.hdfs.server.balancer.TestBalancer +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/5708//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/5708//console This message is automatically generated.
        Hide
        Arpit Agarwal added a comment -

        Thanks for reviewing Eric.

        I apologize for not asking whether you were already working on it.

        Show
        Arpit Agarwal added a comment - Thanks for reviewing Eric. I apologize for not asking whether you were already working on it.
        Hide
        Eric Sirianni added a comment -

        +1 for the patch. I've made a similar change internally and can confirm that it fixes the issue we are seeing in our FsDatasetSpi plugin.

        Show
        Eric Sirianni added a comment - +1 for the patch. I've made a similar change internally and can confirm that it fixes the issue we are seeing in our FsDatasetSpi plugin.
        Hide
        Arpit Agarwal added a comment -

        Patch attached. I made this a sub-task of HDFS-2832 since the fix depends on those changes, and to make it easier to track for any subsequent merge work.

        Show
        Arpit Agarwal added a comment - Patch attached. I made this a sub-task of HDFS-2832 since the fix depends on those changes, and to make it easier to track for any subsequent merge work.
        Hide
        Arpit Agarwal added a comment -

        The same issue exists in 2.2 except that the Datanode identifier is not a UUID. I changed the affected version back to 2.2 which Eric used when filing.

        Show
        Arpit Agarwal added a comment - The same issue exists in 2.2 except that the Datanode identifier is not a UUID. I changed the affected version back to 2.2 which Eric used when filing.
        Hide
        Arpit Agarwal added a comment -

        The javadoc for DataNode#initStorage does not appear to match the code.

        /**
           * Initializes the {@link #data}. The initialization is done only once, when
           * handshake with the the first namenode is completed.
           */
          private void initStorage(final NamespaceInfo nsInfo) throws IOException {
        

        However initStorage is invoked before the handshake completes, in BPServiceActor#connectToNNAndHandshake

            // Verify that this matches the other NN in this HA pair.
            // This also initializes our block pool in the DN if we are
            // the first NN connection for this BP.
            bpos.verifyAndSetNamespaceInfo(nsInfo);    <<<--- Calls initStorage.
            
            // Second phase of the handshake with the NN.
            register();
        

        I am not sure if we need to reorder the calls. Would need to look at this further.

        Show
        Arpit Agarwal added a comment - The javadoc for DataNode#initStorage does not appear to match the code. /** * Initializes the {@link #data}. The initialization is done only once, when * handshake with the the first namenode is completed. */ private void initStorage( final NamespaceInfo nsInfo) throws IOException { However initStorage is invoked before the handshake completes, in BPServiceActor#connectToNNAndHandshake // Verify that this matches the other NN in this HA pair. // This also initializes our block pool in the DN if we are // the first NN connection for this BP. bpos.verifyAndSetNamespaceInfo(nsInfo); <<<--- Calls initStorage. // Second phase of the handshake with the NN. register(); I am not sure if we need to reorder the calls. Would need to look at this further.

          People

          • Assignee:
            Arpit Agarwal
            Reporter:
            Eric Sirianni
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development