Hive
  1. Hive
  2. HIVE-7497

Fix some default values in HiveConf

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.14.0
    • Component/s: None
    • Labels:
      None

      Description

      HIVE-5160 resolves an env variable at runtime via calling System.getenv(). As long as the variable is not defined when you run the build null is returned and the path is not placed in the hive-default,template. However if it is defined it will populate hive-default.template with a path which will be different based on the user running the build. We should use ${system:HIVE_CONF_DIR} instead.

      1. HIVE-7497.1.patch
        2 kB
        Dong Chen
      2. HIVE-7497.patch
        2 kB
        Dong Chen

        Issue Links

          Activity

          Hide
          Dong Chen added a comment -

          This patch changes the property default to $

          {env:HIVE_CONF_DIR}

          Also rename the property to "hive.server2.global.init.file.location" for consistency.

          Show
          Dong Chen added a comment - This patch changes the property default to $ {env:HIVE_CONF_DIR} Also rename the property to "hive.server2.global.init.file.location" for consistency.
          Hide
          Dong Chen added a comment -

          Hi, Brock, do you think shall we use ${env:HIVE_CONF_DIR} instead of ${system:HIVE_CONF_DIR} ?

          In HIVE-6037, the property default values are changed like below:
          System.getProperty("xxx") to "${system:xxx}"
          System.getenv("xxx") to "${system:xxx}"

          Is it better to make the 2nd case to "${env:xxx}" ? Otherwise, the property may not get right env value in runtime.

          If yes, I think the property HIVEHWIWARFILE may also need change like HIVE_GLOBAL_INIT_FILE_LOCATION.
          In old HiveConf.java, its value is System.getenv("HWI_WAR_FILE")). And in new version, it is ${system:HWI_WAR_FILE}

          Show
          Dong Chen added a comment - Hi, Brock, do you think shall we use ${env:HIVE_CONF_DIR} instead of ${system:HIVE_CONF_DIR} ? In HIVE-6037 , the property default values are changed like below: System.getProperty("xxx") to "${system:xxx}" System.getenv("xxx") to "${system:xxx}" Is it better to make the 2nd case to "${env:xxx}" ? Otherwise, the property may not get right env value in runtime. If yes, I think the property HIVEHWIWARFILE may also need change like HIVE_GLOBAL_INIT_FILE_LOCATION. In old HiveConf.java, its value is System.getenv("HWI_WAR_FILE")). And in new version, it is ${system:HWI_WAR_FILE}
          Hide
          Brock Noland added a comment -

          Good point Dong Chen! Navis what do you think. Shouldn't HIVEHWIWARFILE start with the env prefix?

          Show
          Brock Noland added a comment - Good point Dong Chen! Navis what do you think. Shouldn't HIVEHWIWARFILE start with the env prefix?
          Hide
          Hive QA added a comment -

          Overall: -1 at least one tests failed

          Here are the results of testing the latest attachment:
          https://issues.apache.org/jira/secure/attachment/12657564/HIVE-7497.patch

          ERROR: -1 due to 2 failed/errored test(s), 5756 tests executed
          Failed tests:

          org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_ql_rewrite_gbtoidx
          org.apache.hive.hcatalog.pig.TestHCatLoader.testReadDataPrimitiveTypes
          

          Test results: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/44/testReport
          Console output: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/44/console
          Test logs: http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-44/

          Messages:

          Executing org.apache.hive.ptest.execution.PrepPhase
          Executing org.apache.hive.ptest.execution.ExecutionPhase
          Executing org.apache.hive.ptest.execution.ReportingPhase
          Tests exited with: TestsFailedException: 2 tests failed
          

          This message is automatically generated.

          ATTACHMENT ID: 12657564

          Show
          Hive QA added a comment - Overall : -1 at least one tests failed Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12657564/HIVE-7497.patch ERROR: -1 due to 2 failed/errored test(s), 5756 tests executed Failed tests: org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_ql_rewrite_gbtoidx org.apache.hive.hcatalog.pig.TestHCatLoader.testReadDataPrimitiveTypes Test results: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/44/testReport Console output: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/44/console Test logs: http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-44/ Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 2 tests failed This message is automatically generated. ATTACHMENT ID: 12657564
          Hide
          Szehon Ho added a comment -

          This looks good, thanks for taking care of it, but I guess we should wait until HIVE-7496, as it is removing the hive-defaults from the source control.

          Show
          Szehon Ho added a comment - This looks good, thanks for taking care of it, but I guess we should wait until HIVE-7496 , as it is removing the hive-defaults from the source control.
          Hide
          Navis added a comment -

          Dong Chen, Brock Noland Seemed apparently a bug by mistake. Could you fix that too?

          Show
          Navis added a comment - Dong Chen , Brock Noland Seemed apparently a bug by mistake. Could you fix that too?
          Hide
          Dong Chen added a comment -

          Update patch:
          rebase latest trunk with HIVE-7496 code. (remove hive-default).
          And change HIVEHWIWARFILE default value to env prefix in HiveConf.java.

          Show
          Dong Chen added a comment - Update patch: rebase latest trunk with HIVE-7496 code. (remove hive-default). And change HIVEHWIWARFILE default value to env prefix in HiveConf.java.
          Hide
          Brock Noland added a comment -

          +1 pending tests

          Thank you!!

          Show
          Brock Noland added a comment - +1 pending tests Thank you!!
          Hide
          Hive QA added a comment -

          Overall: -1 at least one tests failed

          Here are the results of testing the latest attachment:
          https://issues.apache.org/jira/secure/attachment/12658111/HIVE-7497.1.patch

          ERROR: -1 due to 2 failed/errored test(s), 5770 tests executed
          Failed tests:

          org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_ql_rewrite_gbtoidx
          org.apache.hive.jdbc.miniHS2.TestHiveServer2.testConnection
          

          Test results: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/78/testReport
          Console output: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/78/console
          Test logs: http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-78/

          Messages:

          Executing org.apache.hive.ptest.execution.PrepPhase
          Executing org.apache.hive.ptest.execution.ExecutionPhase
          Executing org.apache.hive.ptest.execution.ReportingPhase
          Tests exited with: TestsFailedException: 2 tests failed
          

          This message is automatically generated.

          ATTACHMENT ID: 12658111

          Show
          Hive QA added a comment - Overall : -1 at least one tests failed Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12658111/HIVE-7497.1.patch ERROR: -1 due to 2 failed/errored test(s), 5770 tests executed Failed tests: org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_ql_rewrite_gbtoidx org.apache.hive.jdbc.miniHS2.TestHiveServer2.testConnection Test results: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/78/testReport Console output: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/78/console Test logs: http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-78/ Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 2 tests failed This message is automatically generated. ATTACHMENT ID: 12658111
          Hide
          Navis added a comment -

          Committed to trunk. Thanks for the contribution, Dong Chen.

          Show
          Navis added a comment - Committed to trunk. Thanks for the contribution, Dong Chen.
          Hide
          Lefty Leverenz added a comment - - edited

          This renames configuration parameter hive.global.init.file.location (created in HIVE-5160) to hive.server2.global.init.file.location and revises its description, as well as changing its default from System.getenv("HIVE_CONF_DIR") to "${env:HIVE_CONF_DIR}" and changing the default of configuration parameter hive.hwi.war.file from "${system:HWI_WAR_FILE}" to "${env:HWI_WAR_FILE}".

          hive.server2.global.init.file.location needs to be documented in two places, with version information and a link to this JIRA ticket (Edit: wrong links, these are for hive.hwi.war.file):

          But it's not clear to me whether the default value changes need any user doc updates – do they?

          (Edit: see later comment for correct hive.server2.global.init.file.location doc links.)

          Show
          Lefty Leverenz added a comment - - edited This renames configuration parameter hive.global.init.file.location (created in HIVE-5160 ) to hive.server2.global.init.file.location and revises its description, as well as changing its default from System.getenv("HIVE_CONF_DIR") to "${env:HIVE_CONF_DIR}" and changing the default of configuration parameter hive.hwi.war.file from "${system:HWI_WAR_FILE}" to "${env:HWI_WAR_FILE}". hive.server2.global.init.file.location needs to be documented in two places, with version information and a link to this JIRA ticket (Edit: wrong links, these are for hive.hwi.war.file ): Hive Web Interface – Configuration Configuration Properties – Hive Web Interface (HWI) But it's not clear to me whether the default value changes need any user doc updates – do they? (Edit: see later comment for correct hive.server2.global.init.file.location doc links.)
          Hide
          Dong Chen added a comment -

          Thanks, Lefty.

          Since the default value change of hive.hwi.war.file is only the prefix part (from system: to env and the actual value is the same, I think user doc may not need updates.

          For hive.server2.global.init.file.location, ${HIVE_CONF_DIR} might be good as the configuration parameter default value in user doc. (prefix env is not neccessary in doc)

          Show
          Dong Chen added a comment - Thanks, Lefty. Since the default value change of hive.hwi.war.file is only the prefix part (from system: to env and the actual value is the same, I think user doc may not need updates. For hive.server2.global.init.file.location , ${HIVE_CONF_DIR} might be good as the configuration parameter default value in user doc. (prefix env is not neccessary in doc)
          Hide
          Lefty Leverenz added a comment -

          Good, that makes sense. Thanks Dong Chen.

          (I'd fix your env smiley but it's fun – let the parenthesis remain open.)

          Show
          Lefty Leverenz added a comment - Good, that makes sense. Thanks Dong Chen . (I'd fix your env smiley but it's fun – let the parenthesis remain open.)
          Hide
          Vaibhav Gumashta added a comment -

          Dong Chen I still have one issue with the patch (sorry for coming out so late on this). We should change HIVE_GLOBAL_INIT_FILE_LOCATION to HIVE_SERVER2_GLOBAL_INIT_FILE_LOCATION. This will be in sync with the convention we've followed so far.

          Show
          Vaibhav Gumashta added a comment - Dong Chen I still have one issue with the patch (sorry for coming out so late on this). We should change HIVE_GLOBAL_INIT_FILE_LOCATION to HIVE_SERVER2_GLOBAL_INIT_FILE_LOCATION. This will be in sync with the convention we've followed so far.
          Hide
          Vaibhav Gumashta added a comment -

          Dong Chen If you're ok with it, I can make the change in one of my patches I'm working on.

          Show
          Vaibhav Gumashta added a comment - Dong Chen If you're ok with it, I can make the change in one of my patches I'm working on.
          Hide
          Brock Noland added a comment -

          [~vaibhavgumashta] since this is commited any additional work would be done in a follow on JIRA.

          Show
          Brock Noland added a comment - [~vaibhavgumashta] since this is commited any additional work would be done in a follow on JIRA.
          Hide
          Vaibhav Gumashta added a comment -

          Brock Noland Sure, that's what I intend to do.

          Show
          Vaibhav Gumashta added a comment - Brock Noland Sure, that's what I intend to do.
          Hide
          Dong Chen added a comment -

          Vaibhav Gumashta Thanks for taking care of it. I'm ok with it and please go ahead. Thanks

          Show
          Dong Chen added a comment - Vaibhav Gumashta Thanks for taking care of it. I'm ok with it and please go ahead. Thanks
          Hide
          Lefty Leverenz added a comment -

          Doc note: HIVE-8138 alters the behavior of hive.server2.global.init.file.location (originally hive.global.init.file.location), so use the description in that patch when documenting the parameter in these two wikidocs:

          Show
          Lefty Leverenz added a comment - Doc note: HIVE-8138 alters the behavior of hive.server2.global.init.file.location (originally hive.global.init.file.location ), so use the description in that patch when documenting the parameter in these two wikidocs: Configuration Properties – HiveServer2 Setting Up HiveServer2 – How to Configure
          Hide
          Szehon Ho added a comment -

          Covered by HIVE-5160

          Show
          Szehon Ho added a comment - Covered by HIVE-5160
          Hide
          Thejas M Nair added a comment -

          This has been fixed in 0.14 release. Please open new jira if you see any issues.

          Show
          Thejas M Nair added a comment - This has been fixed in 0.14 release. Please open new jira if you see any issues.

            People

            • Assignee:
              Dong Chen
              Reporter:
              Brock Noland
            • Votes:
              0 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development