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

JobTracker.getJobCounters() should not hold JobTracker lock while calling JobInProgress.getCounters()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0, 0.23.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      JobTracker.getJobCounter() will lock JobTracker and call JobInProgress.getCounters().
      JobInProgress.getCounters() can be very expensive because it aggregates all the task counters.
      We found that from the JobTracker jstacks that this method is one of the bottleneck of the JobTracker performance.

      JobInProgress.getCounters() should be able to be called out side the JobTracker lock because it already has JobInProgress lock.
      For example, it is used by jobdetails.jsp without a JobTracker lock.

      1. 2026.1.patch
        3 kB
        Joydeep Sen Sarma
      2. MAPREDUCE-2026.txt
        1 kB
        Scott Chen

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #643 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/643/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #643 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk/643/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-22-branch #33 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/33/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-22-branch #33 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-22-branch/33/ )
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #596 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/596/)
          MAPREDUCE-2026. Make JobTracker.getJobCounters() and
          JobInProgress.getCounters() aquire locks in a shorter time period.
          (Joydeep Sen Sarma via schen)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #596 (See https://hudson.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/596/ ) MAPREDUCE-2026 . Make JobTracker.getJobCounters() and JobInProgress.getCounters() aquire locks in a shorter time period. (Joydeep Sen Sarma via schen)
          Hide
          Scott Chen added a comment -

          I just committed this. Thanks Joydeep.

          Show
          Scott Chen added a comment - I just committed this. Thanks Joydeep.
          Hide
          Scott Chen added a comment -

          +1 Patch looks good. And we have been using this in production for a while.
          I will commit this.

          Show
          Scott Chen added a comment - +1 Patch looks good. And we have been using this in production for a while. I will commit this.
          Hide
          Joydeep Sen Sarma added a comment -

          yeah - i think we have this patch in production for many months now - let me regenerate the patch since it's been a long while.

          Show
          Joydeep Sen Sarma added a comment - yeah - i think we have this patch in production for many months now - let me regenerate the patch since it's been a long while.
          Hide
          Konstantin Shvachko added a comment -

          Scot, Joydeep. This seems to be an important fix for 0.22. Do you think it is ready to be committed?

          Show
          Konstantin Shvachko added a comment - Scot, Joydeep. This seems to be an important fix for 0.22. Do you think it is ready to be committed?
          Hide
          Liyin Liang added a comment -

          Hi Joydeep, your patch moved incrementTaskCounters out of lock of JobInProgress in function getCounters(). Should we do the same thing to function getMapCounters() and getReduceCounters()?

          Show
          Liyin Liang added a comment - Hi Joydeep, your patch moved incrementTaskCounters out of lock of JobInProgress in function getCounters(). Should we do the same thing to function getMapCounters() and getReduceCounters()?
          Hide
          Joydeep Sen Sarma added a comment -

          For this particular patch:

          [exec] Please justify why no new tests are needed for this patch.

          • the patch improves performance of existing functionality - the tests for those existing functionality (getCounters) will test whether this patch is working correctly. there is no new functionality or change in behavior of existing functionality
          • it's impossible to trigger specific race conditions (to test changes in locking) without having support for fault/sleep injection in hadoop code base.

          [exec] Also please list what manual steps were performed to verify this patch.

          i have already mentioned this - this patch has been in production in all Facebook Hadoop clusters for a couple of months now.

          thanks for taking a look.

          Show
          Joydeep Sen Sarma added a comment - For this particular patch: [exec] Please justify why no new tests are needed for this patch. the patch improves performance of existing functionality - the tests for those existing functionality (getCounters) will test whether this patch is working correctly. there is no new functionality or change in behavior of existing functionality it's impossible to trigger specific race conditions (to test changes in locking) without having support for fault/sleep injection in hadoop code base. [exec] Also please list what manual steps were performed to verify this patch. i have already mentioned this - this patch has been in production in all Facebook Hadoop clusters for a couple of months now. thanks for taking a look.
          Hide
          Nigel Daley added a comment -

          Hi Joydeep, I see you have a number of issues where this request seems to be ignored:

          [exec] Please justify why no new tests are needed for this patch.
          [exec] Also please list what manual steps were performed to verify this patch.

          Can you please provide this info for your patches?

          Thanks!

          Show
          Nigel Daley added a comment - Hi Joydeep, I see you have a number of issues where this request seems to be ignored: [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. Can you please provide this info for your patches? Thanks!
          Hide
          Joydeep Sen Sarma added a comment -

          hadoopQA output below. this has been running on a production cluster for a week or so:

          [exec] BUILD SUCCESSFUL
          [exec] Total time: 1 minute 17 seconds
          [exec]
          [exec]
          [exec]
          [exec]
          [exec] -1 overall.
          [exec]
          [exec] +1 @author. The patch does not contain any @author tags.
          [exec]
          [exec] -1 tests included. The patch doesn't appear to include any new or modified tests.
          [exec] Please justify why no new tests are needed for this patch.
          [exec] Also please list what manual steps were performed to verify this patch.
          [exec]
          [exec] +1 javadoc. The javadoc tool did not generate any warning messages.
          [exec]
          [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
          [exec]
          [exec] +1 system tests framework. The patch passed system tests framework compile.
          [exec]
          [exec]
          [exec]
          [exec]
          [exec] ======================================================================
          [exec] ======================================================================
          [exec] Finished build.
          [exec] ======================================================================
          [exec] ======================================================================

          Show
          Joydeep Sen Sarma added a comment - hadoopQA output below. this has been running on a production cluster for a week or so: [exec] BUILD SUCCESSFUL [exec] Total time: 1 minute 17 seconds [exec] [exec] [exec] [exec] [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no new tests are needed for this patch. [exec] Also please list what manual steps were performed to verify this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] +1 system tests framework. The patch passed system tests framework compile. [exec] [exec] [exec] [exec] [exec] ====================================================================== [exec] ====================================================================== [exec] Finished build. [exec] ====================================================================== [exec] ======================================================================
          Hide
          Scott Chen added a comment -

          Or what if we launch a separate thread to do counters compute?

          We also thought about this idea of updating counters in a background thread.
          I just created MAPREDUCE-2131 for it.

          Show
          Scott Chen added a comment - Or what if we launch a separate thread to do counters compute? We also thought about this idea of updating counters in a background thread. I just created MAPREDUCE-2131 for it.
          Hide
          Joydeep Sen Sarma added a comment -

          if the client doesn't want updated getCounters - they can reduce the polling interval (for updating the counters). this can be a change made in the jsp code (if that is generating most of the calls in your case).
          for this reason - we didn't put any caching in the jobtracker. (in our case - most of the calls come from the hive clients and the poll interval is already configurable for hive).

          might be worth following up on a new jira?

          Show
          Joydeep Sen Sarma added a comment - if the client doesn't want updated getCounters - they can reduce the polling interval (for updating the counters). this can be a change made in the jsp code (if that is generating most of the calls in your case). for this reason - we didn't put any caching in the jobtracker. (in our case - most of the calls come from the hive clients and the poll interval is already configurable for hive). might be worth following up on a new jira?
          Hide
          luoli added a comment -

          hi Scott and Joydeep, what the patch does is really what we have thought, moreover, the getCounters method will still consume some cpu time and it will still synchronized the JIP, what if we add a call time interval of getCounters? For example, call of getCounters return the result that got like ten seconds ago, and if the call time from the last call exceed ten seconds we recompute the counters? Or what if we launch a separate thread to do counters compute? These are XiaoKang and I have discussed, what do you think?

          Show
          luoli added a comment - hi Scott and Joydeep, what the patch does is really what we have thought, moreover, the getCounters method will still consume some cpu time and it will still synchronized the JIP, what if we add a call time interval of getCounters? For example, call of getCounters return the result that got like ten seconds ago, and if the call time from the last call exceed ten seconds we recompute the counters? Or what if we launch a separate thread to do counters compute? These are XiaoKang and I have discussed, what do you think?
          Hide
          Kang Xiao added a comment -

          +1 even JIP lock is removed for aggregating tips counters

          > From our cluster, abount 26% of jobtracker's lock taken by the jsp access.
          In fact, we decrease lock time by cache getCounters() result. During job is running getCounters are only 1min fresh while it will be completely accurate when job is finished. However this just decrease the invokation for aggregating tips counters and JIP is still in lock when getCounters() is called if the cache is out of 1min.

          Maybe we can combine getCounters() cache and no-lock for aggregating tips counters, than both CPU and lock consumptioncan be decreased. Is that OK?

          Show
          Kang Xiao added a comment - +1 even JIP lock is removed for aggregating tips counters > From our cluster, abount 26% of jobtracker's lock taken by the jsp access. In fact, we decrease lock time by cache getCounters() result. During job is running getCounters are only 1min fresh while it will be completely accurate when job is finished. However this just decrease the invokation for aggregating tips counters and JIP is still in lock when getCounters() is called if the cache is out of 1min. Maybe we can combine getCounters() cache and no-lock for aggregating tips counters, than both CPU and lock consumptioncan be decreased. Is that OK?
          Hide
          Joydeep Sen Sarma added a comment -

          combines a couple patches Scott and I had worked on for higher getCounters concurrency.

          This trades off concurrency for consistency. The value of getCounters for a job is not a point in time summary across all TIP counters. This seemed reasonable because when a job is running - tip counters are constantly changing and the values held in JT/JIP may themselves be outofdate (wrt the actual status of the task running on a different machine).

          However - if a job is finished - then getCounters will be completely accurate. (Intuitively - this would seem to be the consistency clients care about).

          Show
          Joydeep Sen Sarma added a comment - combines a couple patches Scott and I had worked on for higher getCounters concurrency. This trades off concurrency for consistency. The value of getCounters for a job is not a point in time summary across all TIP counters. This seemed reasonable because when a job is running - tip counters are constantly changing and the values held in JT/JIP may themselves be outofdate (wrt the actual status of the task running on a different machine). However - if a job is finished - then getCounters will be completely accurate. (Intuitively - this would seem to be the consistency clients care about).
          Hide
          dhruba borthakur added a comment -

          > From our cluster, abount 26% of jobtracker's lock taken by the jsp access.

          This is a really good observation. we should measure this for our cluster too. If this is the case, do you think a reader/writer lock (instead of a synchronized section) might help performance?

          Show
          dhruba borthakur added a comment - > From our cluster, abount 26% of jobtracker's lock taken by the jsp access. This is a really good observation. we should measure this for our cluster too. If this is the case, do you think a reader/writer lock (instead of a synchronized section) might help performance?
          Hide
          luoli added a comment -

          We use our own hadoop release and we found the same problem because our cluster is using by lots of application developers and most of them like to refresh the jobtracker webui to get the real-time situation of their jobs,specially the jobdetail.jsp. This page want to getCounters of a JIP and will lock the JIP object, the more jobdetail page accessed by people , jobtracker get more inefficient. From our cluster, abount 26% of jobtracker's lock taken by the jsp access.

          Show
          luoli added a comment - We use our own hadoop release and we found the same problem because our cluster is using by lots of application developers and most of them like to refresh the jobtracker webui to get the real-time situation of their jobs,specially the jobdetail.jsp. This page want to getCounters of a JIP and will lock the JIP object, the more jobdetail page accessed by people , jobtracker get more inefficient. From our cluster, abount 26% of jobtracker's lock taken by the jsp access.
          Hide
          Scott Chen added a comment -

          Actually, Joydeep has a better fix for this. Let me assign this to him.

          Show
          Scott Chen added a comment - Actually, Joydeep has a better fix for this. Let me assign this to him.
          Hide
          Scott Chen added a comment -

          In the jstacks we have taken on our cluster, 20% of the jobtracker lock is taken by getJobCounters().
          So this should improve the performance a lot.

          Show
          Scott Chen added a comment - In the jstacks we have taken on our cluster, 20% of the jobtracker lock is taken by getJobCounters(). So this should improve the performance a lot.

            People

            • Assignee:
              Joydeep Sen Sarma
              Reporter:
              Scott Chen
            • Votes:
              1 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development