New issue
Advanced search Search tips

Issue 8742 link

Starred by 5 users

Issue metadata

Status: Released
Owner:
Closed: Apr 2018
Cc:
Components:



Sign in to add a comment

Infinite busy loop in IntraLineLoader.combineLineEdits

Reported by tsuna...@gmail.com, Apr 12 2018

Issue description

Affected Version: 2.15.1

What steps will reproduce the problem?
Not sure exactly.

What is the expected output?
Gerrit shouldn't be pegging the CPU all the time

What do you see instead?
Gerrit is pegging all the available cores all the time

Please provide any additional information below.

I'm seeing a bunch of threads that all look like this one:

"Diff-65" #699 daemon prio=5 os_prio=0 tid=0x00007fd1e803b800 nid=0x5c69 runnable [0x00007fd2cf9f8000]
   java.lang.Thread.State: RUNNABLE
        at com.google.common.collect.RegularImmutableSet.contains(RegularImmutableSet.java:61)
        at com.google.gerrit.server.patch.IntraLineLoader.combineLineEdits(IntraLineLoader.java:271)
        at com.google.gerrit.server.patch.IntraLineLoader.compute(IntraLineLoader.java:116)
        at com.google.gerrit.server.patch.IntraLineLoader.lambda$call$0(IntraLineLoader.java:82)
        at com.google.gerrit.server.patch.IntraLineLoader$$Lambda$383/608697655.call(Unknown Source)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)

   Locked ownable synchronizers:
        - <0x00000005cf808200> (a java.util.concurrent.ThreadPoolExecutor$Worker)

They are constantly running and causing Gerrit to peg all available cores nonstop.  Looking at the code briefly I noticed that this is in a for loop that only conditionally increments the iterator "j".  It seems to me like the loop can potentially busy loop if "j++" never gets executed for some reason.

This change in 2.15.1 looks like it could be the cause: https://github.com/GerritCodeReview/gerrit/commit/d10c18f0c7900f16175052f2a6350ca07e70c8d3#diff-9d159e8af7f3ff356c974475e8caf42f

+      if (editsDueToRebase.contains(c) || editsDueToRebase.contains(n)) {
+        // Don't combine any edits which were identified as being introduced by a rebase as we would
+        // lose that information because of the combination.
+        continue;
+      }

Not sure though, I'm not familiar with Gerrit's code and it's the first time I look at this particular part of the code.  Just a hunch.
 
Project Member

Comment 1 by kaspern@google.com, Apr 12 2018

Cc: alic...@google.com
Components: -PolyGerrit Backend
Project Member

Comment 2 by alic...@google.com, Apr 13 2018

Owner: alic...@google.com
Status: Started (was: New)
You're right. It's an infinite loop. The test which should have caught this also has a bug. :-(
Project Member

Comment 3 by alic...@google.com, Apr 13 2018

Status: ChangeUnderReview (was: Started)
Fix: https://gerrit-review.googlesource.com/c/gerrit/+/172151

What I forgot previously: Thank you for the bug report. It helped a lot.
Labels: FixedIn-2.15.2
Status: Submitted (was: ChangeUnderReview)
Project Member

Comment 5 by logan@google.com, Apr 17 2018

Labels: Performance

Comment 6 by tsuna...@gmail.com, May 23 2018

Hi guys,
Just checking in on what's the ETA for 2.15.2 to get this fix out, we hit it almost daily and I'm trying to decide whether I wait for the release or roll out a build of the the stable-2.15 branch.  Thanks!

Comment 7 by tsuna...@gmail.com, May 26 2018

Deployed the release today, thanks!
Project Member

Comment 8 by gertvdijk@gmail.com, Aug 30

 Issue 9085  has been merged into this issue.
Project Member

Comment 9 by gertvdijk@gmail.com, Aug 30

Status: Released (was: Submitted)

Sign in to add a comment