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

InputReadyVertexManager not resilient to updates in parallelism

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 0.5.4
    • None
    • None
    • Reviewed

    Description

      It sets up its internal book-keeping upon onVertexStarted() but that book-keeping can be wrong if its parallelism changes after that. It needs to subscribe to Vertex configured event for its source vertices to make sure that all changes have been made before initializing its internal state.

      Attachments

        1. TEZ-2011.1.patch
          15 kB
          Bikas Saha
        2. TEZ-2011.2.patch
          14 kB
          Bikas Saha

        Issue Links

          Activity

            bikassaha Bikas Saha added a comment -

            The patch changes the InputReady manager to wait for CONFIGURED signal from all source vertices and its own vertex to start before taking any action. Thus any changes in parallelism in those vertices will take effect before its data structures are initialized and so the calculations will be correct. The logic is essentially the same as before but delayed till all configured signals are received.
            Updated tests for different cases. rajesh.balamohan Please review. Currently this is not an issue since the AM is essentially locked on the central dispatcher. But after TEZ-1914 this may be an issue.

            bikassaha Bikas Saha added a comment - The patch changes the InputReady manager to wait for CONFIGURED signal from all source vertices and its own vertex to start before taking any action. Thus any changes in parallelism in those vertices will take effect before its data structures are initialized and so the calculations will be correct. The logic is essentially the same as before but delayed till all configured signals are received. Updated tests for different cases. rajesh.balamohan Please review. Currently this is not an issue since the AM is essentially locked on the central dispatcher. But after TEZ-1914 this may be an issue.
            hadoopqa Hadoop QA added a comment -

            +1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12695500/TEZ-2011.1.patch
            against master revision a1c8518.

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

            +1 tests included. The patch appears to include 1 new or modified test files.

            +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 2.0.3) 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/101//testReport/
            Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/101//console

            This message is automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12695500/TEZ-2011.1.patch against master revision a1c8518. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +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 2.0.3) 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/101//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/101//console This message is automatically generated.

            lgtm. +1.

            -synchronized in onVertexManagerEventReceived, onRootVertexInitialized, onVertexStateUpdated, onVertexStarted added unintentionally?
            -plz remove unused import before committing

            rajesh.balamohan Rajesh Balamohan added a comment - lgtm. +1. -synchronized in onVertexManagerEventReceived, onRootVertexInitialized, onVertexStateUpdated, onVertexStarted added unintentionally? -plz remove unused import before committing
            bikassaha Bikas Saha added a comment -

            synchronized in onVertexManagerEventReceived, onRootVertexInitialized, onVertexStateUpdated, onVertexStarted added unintentionally

            No. Added them to all public methods to make it consistently locked.

            Thanks for the review!

            bikassaha Bikas Saha added a comment - synchronized in onVertexManagerEventReceived, onRootVertexInitialized, onVertexStateUpdated, onVertexStarted added unintentionally No. Added them to all public methods to make it consistently locked. Thanks for the review!
            bikassaha Bikas Saha added a comment -

            Attaching commit patch.

            bikassaha Bikas Saha added a comment - Attaching commit patch.
            hadoopqa Hadoop QA added a comment -

            +1 overall. Here are the results of testing the latest attachment
            http://issues.apache.org/jira/secure/attachment/12695612/TEZ-2011.2.patch
            against master revision 21bce95.

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

            +1 tests included. The patch appears to include 1 new or modified test files.

            +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 2.0.3) 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/107//testReport/
            Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/107//console

            This message is automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12695612/TEZ-2011.2.patch against master revision 21bce95. +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +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 2.0.3) 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/107//testReport/ Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/107//console This message is automatically generated.
            bikassaha Bikas Saha added a comment -

            Thanks!
            Committed to master
            commit b7268699e684fc4d1ebc00b47d8f1f7e4163bf97
            Author: Bikas Saha <bikas@apache.org>
            Date: Fri Jan 30 17:46:27 2015 -0800

            TEZ-2011. InputReadyVertexManager not resilient to updates in parallelism (bikas)

            bikassaha Bikas Saha added a comment - Thanks! Committed to master commit b7268699e684fc4d1ebc00b47d8f1f7e4163bf97 Author: Bikas Saha <bikas@apache.org> Date: Fri Jan 30 17:46:27 2015 -0800 TEZ-2011 . InputReadyVertexManager not resilient to updates in parallelism (bikas)
            bikassaha Bikas Saha added a comment -

            Committed to branch-0.6
            commit 8cdd988b2549f0aec243a40d3bf7b78b2c9fb4e1
            Author: Bikas Saha <bikas@apache.org>
            Date: Fri Jan 30 17:46:27 2015 -0800

            TEZ-2011. InputReadyVertexManager not resilient to updates in parallelism (bikas)
            (cherry picked from commit b7268699e684fc4d1ebc00b47d8f1f7e4163bf97)

            Committed to branch-0.5
            commit aea2edc7c1c7835a87cef0e614cd23a93fe04976
            Author: Bikas Saha <bikas@apache.org>
            Date: Fri Jan 30 17:46:27 2015 -0800

            TEZ-2011. InputReadyVertexManager not resilient to updates in parallelism (bikas)
            (cherry picked from commit b7268699e684fc4d1ebc00b47d8f1f7e4163bf97)

            bikassaha Bikas Saha added a comment - Committed to branch-0.6 commit 8cdd988b2549f0aec243a40d3bf7b78b2c9fb4e1 Author: Bikas Saha <bikas@apache.org> Date: Fri Jan 30 17:46:27 2015 -0800 TEZ-2011 . InputReadyVertexManager not resilient to updates in parallelism (bikas) (cherry picked from commit b7268699e684fc4d1ebc00b47d8f1f7e4163bf97) Committed to branch-0.5 commit aea2edc7c1c7835a87cef0e614cd23a93fe04976 Author: Bikas Saha <bikas@apache.org> Date: Fri Jan 30 17:46:27 2015 -0800 TEZ-2011 . InputReadyVertexManager not resilient to updates in parallelism (bikas) (cherry picked from commit b7268699e684fc4d1ebc00b47d8f1f7e4163bf97)
            hitesh Hitesh Shah added a comment -

            Closing issue as 0.5.4, 0.6.1 and 0.7.0 have been released.

            hitesh Hitesh Shah added a comment - Closing issue as 0.5.4, 0.6.1 and 0.7.0 have been released.

            People

              bikassaha Bikas Saha
              bikassaha Bikas Saha
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: