Hadoop Common
  1. Hadoop Common
  2. HADOOP-7956

Remove duplicate definition of default config values

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: conf
    • Labels:
      None

      Description

      We define default configuration values in two places:
      #1 The default.xml files (eg core-default.xml)
      #2 The _DEFAULT defines in *Keys.java

      This means the defaults used in the code may or may not be dead code based on whether the config is present in the default xml file. Would be good to define these in one place. Eg:
      #1 Just have the defines in the code and figure out how to make those accessible as a loadable resource (eg could generate the default files from the defines in the KeysPublic* files)
      #2 Remove one of the definitions entirely (possible to live w/o the default files?) or
      #3 Remove the overlap between the code and default files

        Issue Links

          Activity

          Hide
          Robert Joseph Evans added a comment -

          This is further complicated because not all of the defaults are stored in KeysPublic* files. I have seen several locations where the defaults are inlined in the config.get call. These are usually just for code that only one line ever reads the config, but it is still there.

          I am +1 for anything that will help to clean up the configuration.

          Show
          Robert Joseph Evans added a comment - This is further complicated because not all of the defaults are stored in KeysPublic* files. I have seen several locations where the defaults are inlined in the config.get call. These are usually just for code that only one line ever reads the config, but it is still there. I am +1 for anything that will help to clean up the configuration.
          Hide
          Suresh Srinivas added a comment -

          Eli, the default.xml serves as documentation of config parameters. Currently the ones that are not defined in that are hidden configs that used only by internal implementation. The defaults in the code currently serves in case default.xml gets edited. Though the duplication seems not clean, not sure how you can avoid that. Could we throw an exception if the default expected value from the config is missing and get rid of the default defined in the code? That seems like a lot of work.

          Show
          Suresh Srinivas added a comment - Eli, the default.xml serves as documentation of config parameters. Currently the ones that are not defined in that are hidden configs that used only by internal implementation. The defaults in the code currently serves in case default.xml gets edited. Though the duplication seems not clean, not sure how you can avoid that. Could we throw an exception if the default expected value from the config is missing and get rid of the default defined in the code? That seems like a lot of work.
          Hide
          Robert Joseph Evans added a comment -

          Suresh, deleting the version in the code could cause backwards compatibility issues. CommonConfigurationKeysPublic.java is public not only in the java since but also @InterfaceAudience.Public.

          Show
          Robert Joseph Evans added a comment - Suresh, deleting the version in the code could cause backwards compatibility issues. CommonConfigurationKeysPublic.java is public not only in the java since but also @InterfaceAudience.Public.
          Hide
          Eli Collins added a comment -

          Do we need to document them in the default.xml files? The public keys already documented in the javadocs by being public parts of the classes. If we put all the public keys in a single class than (eg put anything missing into CommonConfigurationKeysPublic.java) then I don't see why we need the default.xml files.

          Show
          Eli Collins added a comment - Do we need to document them in the default.xml files? The public keys already documented in the javadocs by being public parts of the classes. If we put all the public keys in a single class than (eg put anything missing into CommonConfigurationKeysPublic.java) then I don't see why we need the default.xml files.
          Hide
          Suresh Srinivas added a comment -

          Assuming you are talking about files such as hdfs-default.xml, yes it does serve as docs for admins on config available and what can be overridden in *-site.xml. I am not sure if javadoc would come handy for such a target audience.

          Show
          Suresh Srinivas added a comment - Assuming you are talking about files such as hdfs-default.xml, yes it does serve as docs for admins on config available and what can be overridden in *-site.xml. I am not sure if javadoc would come handy for such a target audience.
          Hide
          Eli Collins added a comment -

          How about:
          #1 Generating the default.xml files from the PublicKeys.java files? or
          #2 Making the default.xml files just documentation (don't load them as resources, rely on the values defined in the source which IMO should be authoritative)

          Show
          Eli Collins added a comment - How about: #1 Generating the default.xml files from the PublicKeys.java files? or #2 Making the default.xml files just documentation (don't load them as resources, rely on the values defined in the source which IMO should be authoritative)
          Hide
          Harsh J added a comment -

          I like #2.

          #1 has a chance of breaking some behavior, given our reconfiguration history.

          One I can think of, and came up recently from Koji on the ML, is the MR child opts vs. map/reduce specific child opts – if both get present in default.xml, then it voids the former, breaking behavior even if the user wants to specify it.

          Show
          Harsh J added a comment - I like #2. #1 has a chance of breaking some behavior, given our reconfiguration history. One I can think of, and came up recently from Koji on the ML, is the MR child opts vs. map/reduce specific child opts – if both get present in default.xml, then it voids the former, breaking behavior even if the user wants to specify it.
          Hide
          Eli Collins added a comment -

          I like #2 as well. We can verify that the defaults in the code match up with the defaults in the xml files when we make the change.

          Show
          Eli Collins added a comment - I like #2 as well. We can verify that the defaults in the code match up with the defaults in the xml files when we make the change.

            People

            • Assignee:
              Unassigned
              Reporter:
              Eli Collins
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:

                Development