Uploaded image for project: 'Apache Tez'
  1. Apache Tez
  2. TEZ-3223

Support a NullHistoryLogger to disable history logging if needed.

Details

    • Bug
    • Status: Closed
    • Trivial
    • Resolution: Fixed
    • None
    • 0.7.2, 0.9.0, 0.8.4
    • None

    Attachments

      1. TEZ-3223.1.patch
        2 kB
        Hitesh Shah
      2. TEZ-3223.2.patch
        2 kB
        Hitesh Shah
      3. TEZ-3223.3.patch
        2 kB
        Hitesh Shah

      Activity

        tezqa TezQA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12810697/TEZ-3223.1.patch
        against master revision 7f699f1.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. There were no new javadoc warning messages.

        +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1797//testReport/
        Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1797//console

        This message is automatically generated.

        tezqa TezQA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12810697/TEZ-3223.1.patch against master revision 7f699f1. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 findbugs . The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1797//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1797//console This message is automatically generated.
        hitesh Hitesh Shah added a comment -

        sseth review please?

        hitesh Hitesh Shah added a comment - sseth review please?
        tezqa TezQA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12815838/TEZ-3223.3.patch
        against master revision d7b6eb5.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 javadoc. The javadoc tool appears to have generated 1 warning messages.
        See https://builds.apache.org/job/PreCommit-TEZ-Build/1826//artifact/patchprocess/diffJavadocWarnings.txt for details.

        +1 findbugs. The patch does not introduce any new Findbugs (version 3.0.1) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed these unit tests in :
        org.apache.tez.runtime.library.common.sort.impl.TestPipelinedSorter
        org.apache.tez.runtime.library.common.writers.TestUnorderedPartitionedKVWriter

        The following test timeouts occurred in :
        org.apache.tez.dag.history.ats.acls.TestATSHistoryWithACLs

        Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1826//testReport/
        Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1826//console

        This message is automatically generated.

        tezqa TezQA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12815838/TEZ-3223.3.patch against master revision d7b6eb5. +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. See https://builds.apache.org/job/PreCommit-TEZ-Build/1826//artifact/patchprocess/diffJavadocWarnings.txt for details. +1 findbugs . The patch does not introduce any new Findbugs (version 3.0.1) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 core tests . The patch failed these unit tests in : org.apache.tez.runtime.library.common.sort.impl.TestPipelinedSorter org.apache.tez.runtime.library.common.writers.TestUnorderedPartitionedKVWriter The following test timeouts occurred in : org.apache.tez.dag.history.ats.acls.TestATSHistoryWithACLs Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/1826//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/1826//console This message is automatically generated.
        sseth Siddharth Seth added a comment -

        +1. Looks good.

        Does it make sense to include a simple flag - "disabled" or some such, instead of specifying the entire class name ?

        sseth Siddharth Seth added a comment - +1. Looks good. Does it make sense to include a simple flag - "disabled" or some such, instead of specifying the entire class name ?
        hitesh Hitesh Shah added a comment -

        Does it make sense to include a simple flag - "disabled"

        I considered that approach. I am not sure of the potential future changes to reduce the amount of data we write to ATS and how we control it through some form of a log-level knob hence dropped this option for now.

        hitesh Hitesh Shah added a comment - Does it make sense to include a simple flag - "disabled" I considered that approach. I am not sure of the potential future changes to reduce the amount of data we write to ATS and how we control it through some form of a log-level knob hence dropped this option for now.
        hitesh Hitesh Shah added a comment -

        Will commit shortly. Thanks for the review.

        hitesh Hitesh Shah added a comment - Will commit shortly. Thanks for the review.
        hitesh Hitesh Shah added a comment -

        Committed to all active branches.

        hitesh Hitesh Shah added a comment - Committed to all active branches.

        People

          hitesh Hitesh Shah
          hitesh Hitesh Shah
          Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

            Created:
            Updated:
            Resolved: