Details

    • Hadoop Flags:
      Reviewed

      Description

      Prior to the heterogeneous storage feature, each Datanode had a single storage ID which was generated by the Namenode on first registration. The storage ID used fixed Datanode identifiers like IP address and port, so that in a federated cluster, for example, all NameNodes would generate the same storage ID.

      With Heterogeneous storage, we have replaced the storage ID with a per-datanode identifier called the Datanode-UUID. The Datanode UUID is also assigned by a NameNode on first registration. In a federated cluster with multiple namenodes, there are two ways to ensure a unique Datanode UUID allocation:

      1. Synchronize initial registration requests from the BPServiceActors. If a Datanode UUID is already assigned we don't need to synchronize.
      2. The datanode assigns itself a UUID on initialization.
      1. h5448.04.patch
        4 kB
        Arpit Agarwal
      2. h5448.04.addendum.patch
        0.8 kB
        Arpit Agarwal
      3. h5448.03.patch
        4 kB
        Arpit Agarwal
      4. h5448.01.patch
        3 kB
        Arpit Agarwal

        Activity

        Hide
        Arpit Agarwal added a comment -

        A preliminary patch to get any thoughts on the approach.

        This just serializes multiple registrations with a yield between retry attempts. We can probably avoid the serialization for all registrations after the first one.

        Show
        Arpit Agarwal added a comment - A preliminary patch to get any thoughts on the approach. This just serializes multiple registrations with a yield between retry attempts. We can probably avoid the serialization for all registrations after the first one.
        Hide
        Arpit Agarwal added a comment -

        Updated the patch to avoid serialization except for the first time a given Datanode registers.

        All subsequent registrations behave as before.

        Show
        Arpit Agarwal added a comment - Updated the patch to avoid serialization except for the first time a given Datanode registers. All subsequent registrations behave as before.
        Hide
        Eric Sirianni added a comment -

        An issue with relying on the NameNode handshake to assign the UUID is that there is a lengthy period during DataNode initialization where the DatanodeUuid is still NULL. In particular, when the FsDatasetSpi is created in initStorage(), the DatanodeUuid is NULL. The DatanodeUuid is not set afterwards until bpRegistrationSucceeded().

        My team is porting our FsDataset plugin from Hadoop 1.x to Hadoop 2.x and this behavior as changed (in Hadoop 1.x the DatanodeUuid was available when our FsDataset plugin was initialized).

        Due to this condition, I vote for option 2 "The datanode assigns itself a UUID on initialization".

        Show
        Eric Sirianni added a comment - An issue with relying on the NameNode handshake to assign the UUID is that there is a lengthy period during DataNode initialization where the DatanodeUuid is still NULL. In particular, when the FsDatasetSpi is created in initStorage() , the DatanodeUuid is NULL. The DatanodeUuid is not set afterwards until bpRegistrationSucceeded() . My team is porting our FsDataset plugin from Hadoop 1.x to Hadoop 2.x and this behavior as changed (in Hadoop 1.x the DatanodeUuid was available when our FsDataset plugin was initialized). Due to this condition, I vote for option 2 "The datanode assigns itself a UUID on initialization".
        Hide
        Tsz Wo Nicholas Sze added a comment -

        It seems that option 2 is simpler. Is there any drawback?

        Show
        Tsz Wo Nicholas Sze added a comment - It seems that option 2 is simpler. Is there any drawback?
        Hide
        Arpit Agarwal added a comment -

        No drawback, unless we want to let NameNodes manage the UUID assignment.

        An issue with relying on the NameNode handshake to assign the UUID is that there is a lengthy period during DataNode initialization where the DatanodeUuid is still NULL. In particular, when the FsDatasetSpi is created in initStorage(), the DatanodeUuid is NULL. The DatanodeUuid is not set afterwards until bpRegistrationSucceeded().

        Eric Sirianni, I believe this is not very different from today where the storageID remains unassigned until after the first handshake.

        Show
        Arpit Agarwal added a comment - No drawback, unless we want to let NameNodes manage the UUID assignment. An issue with relying on the NameNode handshake to assign the UUID is that there is a lengthy period during DataNode initialization where the DatanodeUuid is still NULL. In particular, when the FsDatasetSpi is created in initStorage(), the DatanodeUuid is NULL. The DatanodeUuid is not set afterwards until bpRegistrationSucceeded(). Eric Sirianni , I believe this is not very different from today where the storageID remains unassigned until after the first handshake.
        Hide
        Arpit Agarwal added a comment -

        Updated patch to generate the id on the datanode.

        Show
        Arpit Agarwal added a comment - Updated patch to generate the id on the datanode.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        +1 patch looks good.

        Show
        Tsz Wo Nicholas Sze added a comment - +1 patch looks good.
        Hide
        Arpit Agarwal added a comment -

        Thanks Nicholas. I committed this to branch HDFS-2832.

        Show
        Arpit Agarwal added a comment - Thanks Nicholas. I committed this to branch HDFS-2832 .
        Hide
        Eric Sirianni added a comment -

        Eric Sirianni, I believe this is not very different from today where the storageID remains unassigned until after the first handshake.

        Arpit Agarwal - true, your change to synchronize the registrations did not create the issue I was seeing - I haven't fully root caused, but that seems to have been caused by the changes to DataNode initialization ordering for namespace federation.

        I chose to comment on this particular JIRA since my observations about NULL UUIDs were relevant in favoring "option 2". Thanks for making the change!

        Show
        Eric Sirianni added a comment - Eric Sirianni, I believe this is not very different from today where the storageID remains unassigned until after the first handshake. Arpit Agarwal - true, your change to synchronize the registrations did not create the issue I was seeing - I haven't fully root caused, but that seems to have been caused by the changes to DataNode initialization ordering for namespace federation. I chose to comment on this particular JIRA since my observations about NULL UUIDs were relevant in favoring "option 2". Thanks for making the change!
        Hide
        Eric Sirianni added a comment -

        Arpit - I think your patch causes a compile error:

        [ERROR] hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java:[337,34] error: unreported exception IOException; must be caught or declared to be thrown
        

        Seems to be caused by this change:

        +  DatanodeRegistration createBPRegistration(NamespaceInfo nsInfo)
        +      throws IOException {
        
        Show
        Eric Sirianni added a comment - Arpit - I think your patch causes a compile error: [ERROR] hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/BPOfferService.java:[337,34] error: unreported exception IOException; must be caught or declared to be thrown Seems to be caused by this change: + DatanodeRegistration createBPRegistration(NamespaceInfo nsInfo) + throws IOException {
        Hide
        Arpit Agarwal added a comment -

        I'm sorry, the error was only triggered on a clean build.

        Thanks for the catch Eric, can you please review the simple fix?

        Show
        Arpit Agarwal added a comment - I'm sorry, the error was only triggered on a clean build. Thanks for the catch Eric, can you please review the simple fix?
        Hide
        Eric Sirianni added a comment -

        +1 patch looks good. I made the same change in my environment and the code compiles successfully.

        Show
        Eric Sirianni added a comment - +1 patch looks good. I made the same change in my environment and the code compiles successfully.
        Hide
        Arpit Agarwal added a comment -

        Thanks Eric, I committed the fix to branch HDFS-2832.

        Show
        Arpit Agarwal added a comment - Thanks Eric, I committed the fix to branch HDFS-2832 .
        Hide
        Eric Sirianni added a comment -

        Arpit - thanks for the change. The current point of UUID initialization still leaves a lengthy window during DataNode startup where UUID is null:

          private void connectToNNAndHandshake() throws IOException {
        
            // ...
        
        // FsDatasetInitialization happens as a side effect of this call    
            bpos.verifyAndSetNamespaceInfo(nsInfo);
            
        // Datanode.checkDatanodeUuid() happens as a side effect of this call
            register();
          }
        

        Would it be possible to simply initialize the UUID immediately in the DataStorage constructor and then overwrite it from the actual properties file later via setFieldsFromProperties() (in the normal case when this is not the first time the DataNode has started).

        (I realize we've moved a bit now from the original JIRA, but it seems reasonable to take care of this as part of this change)

        Show
        Eric Sirianni added a comment - Arpit - thanks for the change. The current point of UUID initialization still leaves a lengthy window during DataNode startup where UUID is null: private void connectToNNAndHandshake() throws IOException { // ... // FsDatasetInitialization happens as a side effect of this call bpos.verifyAndSetNamespaceInfo(nsInfo); // Datanode.checkDatanodeUuid() happens as a side effect of this call register(); } Would it be possible to simply initialize the UUID immediately in the DataStorage constructor and then overwrite it from the actual properties file later via setFieldsFromProperties() (in the normal case when this is not the first time the DataNode has started). (I realize we've moved a bit now from the original JIRA, but it seems reasonable to take care of this as part of this change)
        Hide
        Arpit Agarwal added a comment -

        Eric, let's discuss this on a separate Jira that is not a sub-task of HDFS-2832. Please feel free to file one.

        Show
        Arpit Agarwal added a comment - Eric, let's discuss this on a separate Jira that is not a sub-task of HDFS-2832 . Please feel free to file one.
        Hide
        Eric Sirianni added a comment -

        Actually, rather than having a potentially temporary UUID in the DataStorage object, (as I suggested above), a better solution would be to initialize the UUID in DataNode.initStorage():

          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();
        
            // ...
          }
        

        Arpit - is there any issue with moving the call to checkDatanodeUuid() to initStorage() as suggested above?

        Show
        Eric Sirianni added a comment - Actually, rather than having a potentially temporary UUID in the DataStorage object, (as I suggested above), a better solution would be to initialize the UUID in DataNode.initStorage() : 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(); // ... } Arpit - is there any issue with moving the call to checkDatanodeUuid() to initStorage() as suggested above?
        Hide
        Eric Sirianni added a comment -

        OK - I've filed HDFS-5454 for the aforementioned DataNode UUID initialization issue.

        Show
        Eric Sirianni added a comment - OK - I've filed HDFS-5454 for the aforementioned DataNode UUID initialization issue.
        Hide
        Konstantin Shvachko added a comment -

        > Prior to the heterogeneous storage feature, each Datanode had a single storage ID which was generated by the Namenode on first registration.

        Is that right? storageIDs were generated by DataNodes, not NameNodes, unless I missed the change.

        Show
        Konstantin Shvachko added a comment - > Prior to the heterogeneous storage feature, each Datanode had a single storage ID which was generated by the Namenode on first registration. Is that right? storageIDs were generated by DataNodes, not NameNodes, unless I missed the change.
        Hide
        Arpit Agarwal added a comment -

        storageID was generated by the NameNode in DatanodeManager#registerDatanode

        From trunk

              // this is a new datanode serving a new data storage
              if ("".equals(nodeReg.getStorageID())) {
                // this data storage has never been registered
                // it is either empty or was created by pre-storageID version of DFS
                nodeReg.setStorageID(newStorageID());
        
        Show
        Arpit Agarwal added a comment - storageID was generated by the NameNode in DatanodeManager#registerDatanode From trunk // this is a new datanode serving a new data storage if ("".equals(nodeReg.getStorageID())) { // this data storage has never been registered // it is either empty or was created by pre-storageID version of DFS nodeReg.setStorageID(newStorageID());
        Hide
        Konstantin Shvachko added a comment -

        Ah yes, this is a legacy and pretty confusing part of code, which remained from the times when DNs could not generate their own storageIDs. Now that they do, see DataStorage.createStorageID(), this code path is just never triggered, because nodeReg.getStorageID() is never empty.
        So in practice NameNode never generates storageIDs for DNs.

        Show
        Konstantin Shvachko added a comment - Ah yes, this is a legacy and pretty confusing part of code, which remained from the times when DNs could not generate their own storageIDs. Now that they do, see DataStorage.createStorageID() , this code path is just never triggered, because nodeReg.getStorageID() is never empty. So in practice NameNode never generates storageIDs for DNs.
        Hide
        Arpit Agarwal added a comment -

        Thanks for the correction, you're right.

        Show
        Arpit Agarwal added a comment - Thanks for the correction, you're right.

          People

          • Assignee:
            Arpit Agarwal
            Reporter:
            Arpit Agarwal
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development