Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Invalid
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      This patch introduces CBO step in SemanticAnalyzer. For now the CostBasedOptimizer is an empty shell.
      The contract between SemAly and CBO is:

      • CBO step is controlled by the 'hive.enable.cbo.flag'.
      • When true Hive SemAly will hand CBO a Hive Operator tree (with operators annotated with stats). If it can CBO will return a better plan in Hive AST form.
      1. HIVE-6439.1.patch
        14 kB
        Harish Butani
      2. HIVE-6439.2.patch
        15 kB
        Harish Butani
      3. HIVE-6439.4.patch
        15 kB
        Harish Butani
      4. HIVE-6439.5.patch
        15 kB
        Harish Butani

        Activity

        Hide
        rhbutani Harish Butani added a comment -

        This has been invalidated by the creation of the cbo branch.

        Show
        rhbutani Harish Butani added a comment - This has been invalidated by the creation of the cbo branch.
        Hide
        rhbutani Harish Butani added a comment -

        yes agreed, let's add this post hive 0.13 branching.

        Show
        rhbutani Harish Butani added a comment - yes agreed, let's add this post hive 0.13 branching.
        Hide
        ashutoshc Ashutosh Chauhan added a comment -

        Is plan to get CBO optimizer work in 0.13 timeframe? If no, I think we may want to delay this till branching 0.13 Otherwise, this will go in 0.13 release with a config which doesn't do anything and thus will be confusing for end users.

        Show
        ashutoshc Ashutosh Chauhan added a comment - Is plan to get CBO optimizer work in 0.13 timeframe? If no, I think we may want to delay this till branching 0.13 Otherwise, this will go in 0.13 release with a config which doesn't do anything and thus will be confusing for end users.
        Hide
        thejas Thejas M Nair added a comment -

        +1 LGTM

        Show
        thejas Thejas M Nair added a comment - +1 LGTM
        Hide
        rhbutani Harish Butani added a comment -

        revert to .5 patch as HIVE-6037 has been reverted.

        Show
        rhbutani Harish Butani added a comment - revert to .5 patch as HIVE-6037 has been reverted.
        Hide
        hiveqa Hive QA added a comment -

        Overall: -1 no tests executed

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

        Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1379/testReport
        Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1379/console

        Messages:

        Executing org.apache.hive.ptest.execution.PrepPhase
        Tests exited with: NonZeroExitCodeException
        Command 'bash /data/hive-ptest/working/scratch/source-prep.sh' failed with exit status 1 and output '+ [[ -n '' ]]
        + export 'ANT_OPTS=-Xmx1g -XX:MaxPermSize=256m '
        + ANT_OPTS='-Xmx1g -XX:MaxPermSize=256m '
        + export 'M2_OPTS=-Xmx1g -XX:MaxPermSize=256m -Dhttp.proxyHost=localhost -Dhttp.proxyPort=3128'
        + M2_OPTS='-Xmx1g -XX:MaxPermSize=256m -Dhttp.proxyHost=localhost -Dhttp.proxyPort=3128'
        + cd /data/hive-ptest/working/
        + tee /data/hive-ptest/logs/PreCommit-HIVE-Build-1379/source-prep.txt
        + [[ false == \t\r\u\e ]]
        + mkdir -p maven ivy
        + [[ svn = \s\v\n ]]
        + [[ -n '' ]]
        + [[ -d apache-svn-trunk-source ]]
        + [[ ! -d apache-svn-trunk-source/.svn ]]
        + [[ ! -d apache-svn-trunk-source ]]
        + cd apache-svn-trunk-source
        + svn revert -R .
        ++ awk '{print $2}'
        ++ egrep -v '^X|^Performing status on external'
        ++ svn status --no-ignore
        + rm -rf
        + svn update
        
        Fetching external item into 'hcatalog/src/test/e2e/harness'
        External at revision 1569331.
        
        At revision 1569331.
        + patchCommandPath=/data/hive-ptest/working/scratch/smart-apply-patch.sh
        + patchFilePath=/data/hive-ptest/working/scratch/build.patch
        + [[ -f /data/hive-ptest/working/scratch/build.patch ]]
        + chmod +x /data/hive-ptest/working/scratch/smart-apply-patch.sh
        + /data/hive-ptest/working/scratch/smart-apply-patch.sh /data/hive-ptest/working/scratch/build.patch
        The patch does not appear to apply with p0, p1, or p2
        + exit 1
        '
        

        This message is automatically generated.

        ATTACHMENT ID: 12629471

        Show
        hiveqa Hive QA added a comment - Overall : -1 no tests executed Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12629471/HIVE-6439.5.patch Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1379/testReport Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1379/console Messages: Executing org.apache.hive.ptest.execution.PrepPhase Tests exited with: NonZeroExitCodeException Command 'bash /data/hive-ptest/working/scratch/source-prep.sh' failed with exit status 1 and output '+ [[ -n '' ]] + export 'ANT_OPTS=-Xmx1g -XX:MaxPermSize=256m ' + ANT_OPTS='-Xmx1g -XX:MaxPermSize=256m ' + export 'M2_OPTS=-Xmx1g -XX:MaxPermSize=256m -Dhttp.proxyHost=localhost -Dhttp.proxyPort=3128' + M2_OPTS='-Xmx1g -XX:MaxPermSize=256m -Dhttp.proxyHost=localhost -Dhttp.proxyPort=3128' + cd /data/hive-ptest/working/ + tee /data/hive-ptest/logs/PreCommit-HIVE-Build-1379/source-prep.txt + [[ false == \t\r\u\e ]] + mkdir -p maven ivy + [[ svn = \s\v\n ]] + [[ -n '' ]] + [[ -d apache-svn-trunk-source ]] + [[ ! -d apache-svn-trunk-source/.svn ]] + [[ ! -d apache-svn-trunk-source ]] + cd apache-svn-trunk-source + svn revert -R . ++ awk '{print $2}' ++ egrep -v '^X|^Performing status on external' ++ svn status --no-ignore + rm -rf + svn update Fetching external item into 'hcatalog/src/test/e2e/harness' External at revision 1569331. At revision 1569331. + patchCommandPath=/data/hive-ptest/working/scratch/smart-apply-patch.sh + patchFilePath=/data/hive-ptest/working/scratch/build.patch + [[ -f /data/hive-ptest/working/scratch/build.patch ]] + chmod +x /data/hive-ptest/working/scratch/smart-apply-patch.sh + /data/hive-ptest/working/scratch/smart-apply-patch.sh /data/hive-ptest/working/scratch/build.patch The patch does not appear to apply with p0, p1, or p2 + exit 1 ' This message is automatically generated. ATTACHMENT ID: 12629471
        Hide
        rhbutani Harish Butani added a comment -

        AS requested by John:

        • raise default for 'hive.cbo.max.joins.supported' to 10
        • fix name in hive-default.xml
        Show
        rhbutani Harish Butani added a comment - AS requested by John: raise default for 'hive.cbo.max.joins.supported' to 10 fix name in hive-default.xml
        Hide
        hiveqa 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/12629326/HIVE-6439.4.patch

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

        org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_bucketmapjoin6
        

        Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1362/testReport
        Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1362/console

        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: 1 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12629326

        Show
        hiveqa 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/12629326/HIVE-6439.4.patch ERROR: -1 due to 1 failed/errored test(s), 5127 tests executed Failed tests: org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_bucketmapjoin6 Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1362/testReport Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1362/console 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: 1 tests failed This message is automatically generated. ATTACHMENT ID: 12629326
        Hide
        brocknoland Brock Noland added a comment -

        Hi Laljo,

        Essentially that is correct. The problem with that code is that it catches Throwable and logs at debug. This means nasty errors such as OOM or internal JVM errors which occur and will not be logged since most users do not log at the debug level in production. I'd suggest not catching throwable and instead catching exception. Catching exception is still a poor coding practice as you will catch all kinds of runtime errors which you do not expect. I am guessing the author is looking to "try CBO" and fail back if a bug is hit. Even in that case, catching exception, we should be logging at a ERROR level unless there is a very good reason not to.

        Show
        brocknoland Brock Noland added a comment - Hi Laljo, Essentially that is correct. The problem with that code is that it catches Throwable and logs at debug. This means nasty errors such as OOM or internal JVM errors which occur and will not be logged since most users do not log at the debug level in production. I'd suggest not catching throwable and instead catching exception. Catching exception is still a poor coding practice as you will catch all kinds of runtime errors which you do not expect. I am guessing the author is looking to "try CBO" and fail back if a bug is hit. Even in that case, catching exception, we should be logging at a ERROR level unless there is a very good reason not to.
        Hide
        hiveqa Hive QA added a comment -

        Overall: +1 all checks pass

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

        SUCCESS: +1 5120 tests passed

        Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1348/testReport
        Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1348/console

        Messages:

        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        

        This message is automatically generated.

        ATTACHMENT ID: 12629280

        Show
        hiveqa Hive QA added a comment - Overall : +1 all checks pass Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12629280/HIVE-6439.2.patch SUCCESS: +1 5120 tests passed Test results: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1348/testReport Console output: http://bigtop01.cloudera.org:8080/job/PreCommit-HIVE-Build/1348/console Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase This message is automatically generated. ATTACHMENT ID: 12629280
        Hide
        rhbutani Harish Butani added a comment -
        Show
        rhbutani Harish Butani added a comment - Review at https://reviews.apache.org/r/18172/
        Hide
        jpullokkaran Laljo John Pullokkaran added a comment -

        @Brock #2 Is the concern that we are swallowing the exception or is it that the log level should be info or warning instead of debug?

        Show
        jpullokkaran Laljo John Pullokkaran added a comment - @Brock #2 Is the concern that we are swallowing the exception or is it that the log level should be info or warning instead of debug?
        Hide
        brocknoland Brock Noland added a comment -

        1) There are many tabs and spacing issues in this patch
        2) We are catching throwable and logging at debug. We should never do this.

        } catch (Throwable t) {
        LOG.debug("CBO failed, skipping CBO", t);
        
        Show
        brocknoland Brock Noland added a comment - 1) There are many tabs and spacing issues in this patch 2) We are catching throwable and logging at debug. We should never do this. } catch (Throwable t) { LOG.debug("CBO failed, skipping CBO", t);
        Hide
        thejas Thejas M Nair added a comment -

        Can you please create a reviewboard link ? Can you also add the config parameters to hive-default.xml.template and give a description there ? Indentation seems to be off at some places, can you also please fix that ?

        Show
        thejas Thejas M Nair added a comment - Can you please create a reviewboard link ? Can you also add the config parameters to hive-default.xml.template and give a description there ? Indentation seems to be off at some places, can you also please fix that ?

          People

          • Assignee:
            rhbutani Harish Butani
            Reporter:
            rhbutani Harish Butani
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development