Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-6418

Regression: DFS_NAMENODE_USER_NAME_KEY missing in trunk

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 3.0.0, 2.5.0
    • Fix Version/s: 2.5.0
    • Component/s: hdfs-client
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Code i have that compiles against HADOOP 2.4 doesn't build against trunk as someone took away DFSConfigKeys.DFS_NAMENODE_USER_NAME_KEY -apparently in HDFS-6181.

      I know the name was obsolete, but anyone who has compiled code using that reference rather than cutting and pasting in the string is going to find their code doesn't work.

      More subtly: that will lead to a link exception trying to run that code on a 2.5+ cluster.

      This is a regression: the old names need to go back in, even if they refer to the new names and are marked as deprecated

      1. h6418_20140619.patch
        3 kB
        Tsz Wo Nicholas Sze

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-Hdfs-trunk #1790 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1790/)
          HDFS-6418. Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536)

          • /hadoop/common/trunk
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-Hdfs-trunk #1790 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1790/ ) HDFS-6418 . Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536 ) /hadoop/common/trunk /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Mapreduce-trunk #1817 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1817/)
          HDFS-6418. Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536)

          • /hadoop/common/trunk
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Mapreduce-trunk #1817 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1817/ ) HDFS-6418 . Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536 ) /hadoop/common/trunk /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hadoop-Yarn-trunk #599 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/599/)
          HDFS-6418. Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536)

          • /hadoop/common/trunk
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Show
          Hudson added a comment - FAILURE: Integrated in Hadoop-Yarn-trunk #599 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/599/ ) HDFS-6418 . Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536 ) /hadoop/common/trunk /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Hide
          Tsz Wo Nicholas Sze added a comment -

          Thanks Steve for testing and committing the patch. Resolving ...

          Show
          Tsz Wo Nicholas Sze added a comment - Thanks Steve for testing and committing the patch. Resolving ...
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #5799 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5799/)
          HDFS-6418. Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536)

          • /hadoop/common/trunk
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java
          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Show
          Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #5799 (See https://builds.apache.org/job/Hadoop-trunk-Commit/5799/ ) HDFS-6418 . Regression: DFS_NAMENODE_USER_NAME_KEY missing (stevel: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1606536 ) /hadoop/common/trunk /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          Hide
          Steve Loughran added a comment -

          patch applied, verify slider builds. Thanks!

          Show
          Steve Loughran added a comment - patch applied, verify slider builds. Thanks!
          Hide
          Steve Loughran added a comment -

          In the long term, Slider should eliminate referencing DFSConfigKeys. In the short term, I don't mind adding back the previously removed constants as deprecated. Sounds good?

          I could eliminate the references -but think that if I used the constants, others would too. I just found it first as I do try to build my code against trunk regularly

          There's no equivalent in HDFS of the public constants offered by Common and YARN. We need something similar with the public stable constant strings; this private-tagged interface could extend it. Moving the constants would be simple matter of then deciding what is public, what is private and pulling up the public ones.

          Show
          Steve Loughran added a comment - In the long term, Slider should eliminate referencing DFSConfigKeys. In the short term, I don't mind adding back the previously removed constants as deprecated. Sounds good? I could eliminate the references -but think that if I used the constants, others would too. I just found it first as I do try to build my code against trunk regularly There's no equivalent in HDFS of the public constants offered by Common and YARN. We need something similar with the public stable constant strings; this private-tagged interface could extend it. Moving the constants would be simple matter of then deciding what is public, what is private and pulling up the public ones.
          Hide
          Steve Loughran added a comment -

          +1 for the patch -thanks.

          Show
          Steve Loughran added a comment - +1 for the patch -thanks.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          h6418_20140619.patch: adds back the previously deleted constants.

          Show
          Tsz Wo Nicholas Sze added a comment - h6418_20140619.patch: adds back the previously deleted constants.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I agree with Suresh and Jing that DFSConfigKeys is not a public API so that it should not be used outside HDFS. On the other hand, the conf property key is a public API. For example,

          //DFSConfigKeys.java
            public static final String  DFS_BLOCK_SIZE_KEY = "dfs.blocksize";
          

          DFS_BLOCK_SIZE_KEY is not a public API. Projects outside HDFS should not use DFSConfigKeys.DFS_BLOCK_SIZE_KEY. However, the value "dfs.blocksize" is a public API as it is defined in hdfs-default.xml.

          In the long term, Slider should eliminate referencing DFSConfigKeys. In the short term, I don't mind adding back the previously removed constants as deprecated. Sounds good?

          Show
          Tsz Wo Nicholas Sze added a comment - I agree with Suresh and Jing that DFSConfigKeys is not a public API so that it should not be used outside HDFS. On the other hand, the conf property key is a public API. For example, //DFSConfigKeys.java public static final String DFS_BLOCK_SIZE_KEY = "dfs.blocksize" ; DFS_BLOCK_SIZE_KEY is not a public API. Projects outside HDFS should not use DFSConfigKeys.DFS_BLOCK_SIZE_KEY. However, the value "dfs.blocksize" is a public API as it is defined in hdfs-default.xml. In the long term, Slider should eliminate referencing DFSConfigKeys. In the short term, I don't mind adding back the previously removed constants as deprecated. Sounds good?
          Hide
          Jing Zhao added a comment -

          DFSConfigKeys becomes the public keyset (for compatibility)

          Currently it feels like to me that a lot of keys defined in DFSConfigKeys should be private to HDFS. If we want to expose some hdfs configuration keys as public to external projects, maybe we should make DFSConfigKeys as private first, and gradually move those public keyset to a new public ConfigKeys class.

          Show
          Jing Zhao added a comment - DFSConfigKeys becomes the public keyset (for compatibility) Currently it feels like to me that a lot of keys defined in DFSConfigKeys should be private to HDFS. If we want to expose some hdfs configuration keys as public to external projects, maybe we should make DFSConfigKeys as private first, and gradually move those public keyset to a new public ConfigKeys class.
          Hide
          Steve Loughran added a comment -

          How about

          1. DFSConfigKeys becomes the public keyset (for compatibility)
          2. a subclass of this, DFSPrivateConfigKeys becomes where private keys go
          3. We add the (deleted) tags but deprecate them.
          4. stuff in trunk that is new and private gets pushed into the private keys, promoted as and when its felt to make them public.

          I can do the creation of the private keys file, revert the deleted keys -enough to help me build/link my code...leaving the choice of new stuff to keep private to others

          Show
          Steve Loughran added a comment - How about DFSConfigKeys becomes the public keyset (for compatibility) a subclass of this, DFSPrivateConfigKeys becomes where private keys go We add the (deleted) tags but deprecate them. stuff in trunk that is new and private gets pushed into the private keys, promoted as and when its felt to make them public. I can do the creation of the private keys file, revert the deleted keys -enough to help me build/link my code...leaving the choice of new stuff to keep private to others
          Hide
          Steve Loughran added a comment -

          -there's no other place in the hdfs codebase that defines the properties for hdfs as constant strings...anyone who doesn't want to cut and paste values is going to link to this.

          Which is preferable?

          1. people cut and pasting strings like {"dfs.replication"}
          2. people importing constants defined in the hadoop source, as is done via YarnConfiguration and CommonConfigurationKeysPublic?

          I may have been unusual in that I tried to use the in-source constants. And I may have (unintentionally) used them despite them being annotated private -but when you do YARN code you end up treating that as a mild hint anyway.

          Options

          1. do nothing, I fix my code to inline the constant in my own constants class. I repeat this for any other imports in my code, as I can no longer be confident that they will remain there. Anyone else who uses the constant finds their code breaks.
          2. Add a deprecated definition of the old name, using the new name as its reference.
          3. action #2, then extract a stable set of constants into a HDFSPublicKeys class for others to use, make this a superclass of the private keys, and encourage people to use these constants in future.

          Now -how are static strings imported into other classes in the compiler? Copied or linked? If copied, code that imports the old definitions will not fail at runtime -only when precompiled. Which would reduce the damage somewhat

          Show
          Steve Loughran added a comment - -there's no other place in the hdfs codebase that defines the properties for hdfs as constant strings...anyone who doesn't want to cut and paste values is going to link to this. Which is preferable? people cut and pasting strings like {"dfs.replication"} people importing constants defined in the hadoop source, as is done via YarnConfiguration and CommonConfigurationKeysPublic ? I may have been unusual in that I tried to use the in-source constants. And I may have (unintentionally) used them despite them being annotated private -but when you do YARN code you end up treating that as a mild hint anyway. Options do nothing, I fix my code to inline the constant in my own constants class. I repeat this for any other imports in my code, as I can no longer be confident that they will remain there. Anyone else who uses the constant finds their code breaks. Add a deprecated definition of the old name, using the new name as its reference. action #2, then extract a stable set of constants into a HDFSPublicKeys class for others to use, make this a superclass of the private keys, and encourage people to use these constants in future. Now -how are static strings imported into other classes in the compiler? Copied or linked? If copied, code that imports the old definitions will not fail at runtime -only when precompiled. Which would reduce the damage somewhat
          Hide
          Suresh Srinivas added a comment -

          I agree with Brandon Li, we should not be using private classes in external projects. Either we should make the class interface audience public or define constants locally in the project.

          Show
          Suresh Srinivas added a comment - I agree with Brandon Li , we should not be using private classes in external projects. Either we should make the class interface audience public or define constants locally in the project.
          Hide
          Brandon Li added a comment -

          DFSConfigKeys is a private class and not intended to be used outside HDFS.
          DFS_NAMENODE_USER_NAME_KEY was renamed to DFS_NAMENODE_KERBEROS_PRINCIPAL_KEY. We add deprecated keys usually when the value changes, but in this case the value is still "dfs.namenode.kerberos.principal".

          Show
          Brandon Li added a comment - DFSConfigKeys is a private class and not intended to be used outside HDFS. DFS_NAMENODE_USER_NAME_KEY was renamed to DFS_NAMENODE_KERBEROS_PRINCIPAL_KEY. We add deprecated keys usually when the value changes, but in this case the value is still "dfs.namenode.kerberos.principal".

            People

            • Assignee:
              Tsz Wo Nicholas Sze
              Reporter:
              Steve Loughran
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development