Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-5066

JobTracker should set a timeout when calling into job.end.notification.url

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1-win, 2.0.3-alpha, 1.3.0
    • Fix Version/s: 1.2.0, 1-win, 2.1.0-beta
    • Component/s: None
    • Labels:
      None

      Description

      In current code, timeout is not specified when JobTracker (JobEndNotifier) calls into the notification URL. When the given URL points to a server that will not respond for a long time, job notifications are completely stuck (given that we have only a single thread processing all notifications). We've seen this cause noticeable delays in job execution in components that rely on job end notifications (like Oozie workflows).

      I propose we introduce a configurable timeout option and set a default to a reasonably small value.

      If we want, we can also introduce a configurable number of workers processing the notification queue (not sure if this is needed though at this point).

      I will prepare a patch soon. Please comment back.

      1. MAPREDUCE-5066.patch
        7 kB
        Ivan Mitic
      2. MAPREDUCE-5066.branch-1-win.patch
        16 kB
        Ivan Mitic
      3. MAPREDUCE-5066.branch-1-win.5.patch
        17 kB
        Ivan Mitic
      4. MAPREDUCE-5066.branch-1-win.4.patch
        17 kB
        Ivan Mitic
      5. MAPREDUCE-5066.branch-1-win.3.patch
        16 kB
        Ivan Mitic
      6. MAPREDUCE-5066.branch-1-win.2.patch
        16 kB
        Ivan Mitic
      7. MAPREDUCE-5066.3.patch
        17 kB
        Ivan Mitic
      8. MAPREDUCE-5066.2.patch
        8 kB
        Ivan Mitic

        Issue Links

          Activity

          Hide
          Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1406 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1406/)
          MAPREDUCE-5066. Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216)

          Result = SUCCESS
          acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1406 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1406/ ) MAPREDUCE-5066 . Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216) Result = SUCCESS acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1379 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1379/)
          MAPREDUCE-5066. Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216)

          Result = FAILURE
          acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1379 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1379/ ) MAPREDUCE-5066 . Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216) Result = FAILURE acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Yarn-trunk #190 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/190/)
          MAPREDUCE-5066. Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216)

          Result = SUCCESS
          acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Show
          Hudson added a comment - Integrated in Hadoop-Yarn-trunk #190 (See https://builds.apache.org/job/Hadoop-Yarn-trunk/190/ ) MAPREDUCE-5066 . Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216) Result = SUCCESS acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Hide
          Arun C Murthy added a comment -

          I just committed this. Thanks Ivan!

          Show
          Arun C Murthy added a comment - I just committed this. Thanks Ivan!
          Hide
          Hudson added a comment -

          Integrated in Hadoop-trunk-Commit #3643 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3643/)
          MAPREDUCE-5066. Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216)

          Result = SUCCESS
          acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216
          Files :

          • /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java
          • /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Show
          Hudson added a comment - Integrated in Hadoop-trunk-Commit #3643 (See https://builds.apache.org/job/Hadoop-trunk-Commit/3643/ ) MAPREDUCE-5066 . Added a timeout for the job.end.notification.url. Contributed by Ivan Mitic. (Revision 1470216) Result = SUCCESS acmurthy : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470216 Files : /hadoop/common/trunk/hadoop-mapreduce-project/CHANGES.txt /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/TestJobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/JobEndNotifier.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRJobConfig.java /hadoop/common/trunk/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/test/java/org/apache/hadoop/mapred/TestJobEndNotifier.java
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12578673/MAPREDUCE-5066.3.patch
          against trunk revision .

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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 hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

          +1 contrib tests. The patch passed contrib unit tests.

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12578673/MAPREDUCE-5066.3.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) 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 hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3528//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3528//console This message is automatically generated.
          Hide
          Ivan Mitic added a comment -

          Attaching updated patches. Arun, let me know it this looks good.

          Thanks

          Show
          Ivan Mitic added a comment - Attaching updated patches. Arun, let me know it this looks good. Thanks
          Hide
          Ivan Mitic added a comment -

          Thanks for the review Arun! Sounds good, let me prepare the updated patch.

          Show
          Ivan Mitic added a comment - Thanks for the review Arun! Sounds good, let me prepare the updated patch.
          Hide
          Arun C Murthy added a comment -

          Ivan Mitic - sorry for the delay. Minor nit: mapred.job.end.notification.timeout should be mapreduce.job.end.notification.timeout; also we need to use the same config in MR2 too.

          Show
          Arun C Murthy added a comment - Ivan Mitic - sorry for the delay. Minor nit: mapred.job.end.notification.timeout should be mapreduce.job.end.notification.timeout; also we need to use the same config in MR2 too.
          Hide
          Ivan Mitic added a comment -

          Arun, did you get a chance to take a look at my latest branch-1/branch-2 patches? Please check my comment from above from some context.

          Thx!

          Show
          Ivan Mitic added a comment - Arun, did you get a chance to take a look at my latest branch-1/branch-2 patches? Please check my comment from above from some context. Thx!
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12576327/MAPREDUCE-5066.2.patch
          against trunk revision .

          +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. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

          +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) 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 hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

          +1 contrib tests. The patch passed contrib unit tests.

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

          This message is automatically generated.

          Show
          Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12576327/MAPREDUCE-5066.2.patch against trunk revision . +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 . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) 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 hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3485//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3485//console This message is automatically generated.
          Hide
          Ivan Mitic added a comment -

          Audit warning fix, missing the Apache header in the new test file.

          Show
          Ivan Mitic added a comment - Audit warning fix, missing the Apache header in the new test file.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12576324/MAPREDUCE-5066.patch
          against trunk revision .

          +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. The javadoc tool did not generate any warning messages.

          +1 eclipse:eclipse. The patch built with eclipse:eclipse.

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

          -1 release audit. The applied patch generated 1 release audit warnings.

          +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3484//testReport/
          Release audit warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3484//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3484//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12576324/MAPREDUCE-5066.patch against trunk revision . +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 . The javadoc tool did not generate any warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. -1 release audit . The applied patch generated 1 release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3484//testReport/ Release audit warnings: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3484//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3484//console This message is automatically generated.
          Hide
          Ivan Mitic added a comment -

          Hi Arun,

          I am attaching branch-1-win/branch-1 and branch-2 compatible patches.

          A few notes on the patches:

          • Fixed a test verification issue in branch-1-win.3.patch
          • branch-1 and branch-1-win patches are fully compatible (and equivalent)
          • Branch-2 codebase changed significantly and I did my best effort to find the appropriate forward patch. There are two implementations of the JobEndNotifier, mapred#JobEndNotifier (based on the one from branch-1 but simplified) and mapreduce.v2.app#JobEndNotifier (new implementation). The former is used in the LocalJobRunner and latter in the MR AppMaster. In my patch I did the following:
            a. Applied the bugfixes to current state of mapred#JobEndNotifier and included the corresponding unittests
            b. Given that mapreduce.v2.app#JobEndNotifier already sets the timeout to 5 seconds, I did the same in mapred#JobEndNotifier. In other words, I did not introduce a config knob that would allow the timeout to be configurable. My reasoning was that in branch-1, people might see 5 second timeout as a regression and might want to change it to a different value. In trunk, given that the timeout is already set to 5 seconds, this should be fine until proved otherwise. Please advise if you think this is needed.
          Show
          Ivan Mitic added a comment - Hi Arun, I am attaching branch-1-win/branch-1 and branch-2 compatible patches. A few notes on the patches: Fixed a test verification issue in branch-1-win.3.patch branch-1 and branch-1-win patches are fully compatible (and equivalent) Branch-2 codebase changed significantly and I did my best effort to find the appropriate forward patch. There are two implementations of the JobEndNotifier, mapred#JobEndNotifier (based on the one from branch-1 but simplified) and mapreduce.v2.app#JobEndNotifier (new implementation). The former is used in the LocalJobRunner and latter in the MR AppMaster. In my patch I did the following: a. Applied the bugfixes to current state of mapred#JobEndNotifier and included the corresponding unittests b. Given that mapreduce.v2.app#JobEndNotifier already sets the timeout to 5 seconds, I did the same in mapred#JobEndNotifier. In other words, I did not introduce a config knob that would allow the timeout to be configurable. My reasoning was that in branch-1, people might see 5 second timeout as a regression and might want to change it to a different value. In trunk, given that the timeout is already set to 5 seconds, this should be fine until proved otherwise. Please advise if you think this is needed.
          Hide
          Ivan Mitic added a comment -

          You'll need to port this to branch-1 and branch-2(trunk).

          Thanks for reviewing Arun! Will attach branch-1 and branch-2 compatible patches in a bit.

          Also, if you could minimize formatting changes it will make it easier to review (for future ref). Thanks!

          Sounds good, thanks. I noticed that formatting was incorrect so I intentionally made this change Will avoid doing formatting changes with bugfixes going forward.

          Show
          Ivan Mitic added a comment - You'll need to port this to branch-1 and branch-2(trunk). Thanks for reviewing Arun! Will attach branch-1 and branch-2 compatible patches in a bit. Also, if you could minimize formatting changes it will make it easier to review (for future ref). Thanks! Sounds good, thanks. I noticed that formatting was incorrect so I intentionally made this change Will avoid doing formatting changes with bugfixes going forward.
          Hide
          Arun C Murthy added a comment -

          Ivan - the patch looks good. You'll need to port this to branch-1 and branch-2(trunk).

          Also, if you could minimize formatting changes it will make it easier to review (for future ref). Thanks!

          Show
          Arun C Murthy added a comment - Ivan - the patch looks good. You'll need to port this to branch-1 and branch-2(trunk). Also, if you could minimize formatting changes it will make it easier to review (for future ref). Thanks!
          Hide
          Ivan Mitic added a comment -

          Minor patch update, factoring common unittest code into utility methods.

          Show
          Ivan Mitic added a comment - Minor patch update, factoring common unittest code into utility methods.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12574085/MAPREDUCE-5066.branch-1-win.patch
          against trunk revision .

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3423//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12574085/MAPREDUCE-5066.branch-1-win.patch against trunk revision . -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/3423//console This message is automatically generated.
          Hide
          Ivan Mitic added a comment -

          Job notification also exists in 2.x which may face the same set of issues.

          Thanks Hitesh, it should be strait forward to rebase the patch for 2.x branch. Will do so once the current patch is reviewed.

          Show
          Ivan Mitic added a comment - Job notification also exists in 2.x which may face the same set of issues. Thanks Hitesh, it should be strait forward to rebase the patch for 2.x branch. Will do so once the current patch is reviewed.
          Hide
          Ivan Mitic added a comment -

          Attaching the branch-1 compatible patch.

          A few notes:

          • Introduced missing unittests for the JobEndNotifier that cover most of its functionality
          • Added a test case that targets the problem from the Jira
          • Fixed a bug in how retry count it computed (we had an extra retry attempt previously)
          Show
          Ivan Mitic added a comment - Attaching the branch-1 compatible patch. A few notes: Introduced missing unittests for the JobEndNotifier that cover most of its functionality Added a test case that targets the problem from the Jira Fixed a bug in how retry count it computed (we had an extra retry attempt previously)
          Hide
          Hitesh Shah added a comment -

          Job notification also exists in 2.x which may face the same set of issues.

          Show
          Hitesh Shah added a comment - Job notification also exists in 2.x which may face the same set of issues.

            People

            • Assignee:
              Ivan Mitic
              Reporter:
              Ivan Mitic
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development