Performance issues on build status updates

[Pistahh]

Hi,

cc @bturner for ideas and suggestions

we started to see a performance issue with our Bitbucket servers, there seems to be correlation with installing version 2.5.0 of the plugin.

We use Bitbucket Data center edition, which means that all the Git repositories are on NFS, which is relatively slow.

There are many hundreds of open pull requests on the server; on build status updates PR Harmony checks all of them if they contain the commit of the status update (to do the automerges). This kicks off many"git rev-list" processes (for prService.streamCommits I guess) that, due to NFS, complete very slowly. On one build status update through the REST API sometimes we see that running all these git commands can take overall 16 seconds).

To make things worse, most projects build many variants which finish roughly the same time, so they all update the build statuses the same time, amplifying the delay - in some cases it takes 20 minutes between the last build status update and its last related "git rev-parse" completing. During this time (due to the Bitbucket internal "ticket" system) other operations can get blocked, people's "git push"-es "hanging" because of the server waiting for a new ticket to execute the post request hooks.

But this could be avoided as we have no projects/repos at all configured for PR Harmony automerges, i.e. all those checks could be completely skipped.

Would it be possible to enhance PR harmony to check the if the repo of the actual PR has any automerge branch configured and skip the commit checks if there are none? Or for a way to just disable the automerge feature?

Thanks,
Istvan

[bturner]

@Pistahh,

I won't say I'm happy to take the lumps for this, but I'll do so regardless. 2.5 introduced a change that moved some processing off of Bitbucket Server's event threads, to prevent starvation for our event subsystem. That's an important and necessary change, because running out of event threads can result in extremely difficult to explain, difficult to debug issues with system behavior.

Unfortunately, it's highlighting a different constrained shared resource, and that's the ExecutorService we make available for add-on developers. That executor is also used to process the pre-receive and post-receive hook callbacks which are part of servicing push operations. So when PR Harmony gets busy, it can block pushes. BSERV-10652 tracks the issue, and a fix for that will be merged today, with point releases to follow. Those point releases will only extend back to Bitbucket Server 5.5.x and newer, though (i.e. only 5.5, 5.6, 5.7 and 5.8 will receive patches). Those still on 4.x or earlier 5.x releases will need to upgrade to get the fix.

One thing that's important, at least in my opinion, is that the improvement from 2.5.0, moving PR Harmony processing off Bitbucket Server's event threads, not be regressed. Fortunately, there should be a relatively straightforward way to keep that improvement while also avoiding this issue, and that's to make a couple changes in the PR Harmony plugin:

• Update atlassian-plugin.xml to drop the <component-import/> for the ExecutorService
• Update AsyncProcessor to have a zero-argument constructor and create its own executor service

public class AsyncProcessor {

 private final ExecutorService executorService;

  public AsyncProcessor() {
    executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(),
        ThreadFactories.namedThreadFactory("pr-harmony", Type.DAEMON));
  }

  public void dispatch(Runnable task) {
    executorService.execute(task);
  }
}

(Cleaned up a few trailing bits from the now-defunct BucketedExecutor approach AsyncProcessor used to use, while I was putting together my example.)

The com.atlassian.util.concurrent.ThreadFactories type is part of atlassian-util-concurrent. If the add-on doesn't already have access to it (it should; try to import it), it can be added with the following Maven dependency:

        <dependency>
            <groupId>com.atlassian.util.concurrent</groupId>
            <artifactId>atlassian-util-concurrent</artifactId>
            <scope>provided</scope>
        </dependency>

(Note: The add-on doesn't need to specify a version; the <scope>import</scope> of the bitbucket-parent POM will provide the right one. (And on that note, I'll point out that Bitbucket Server provides Guava 18, not 21 as this add-on's POM declares.))

This change will make PR Harmony use its own executor for its background work, freeing up the shared executor to better service hook requests. The unfortunate thing about this fix is that, for versions of Bitbucket Server where I've fixed the HookService, I'd actually prefer the PR Harmony add-on (and add-ons in general), use the shared executor, specifically because that helps constrain background processing across add-ons, which helps manage load on the Bitbucket Server instance and prevent the accumulated background processing of all add-ons from negatively impacting overall performance.

I'm happy to review a change, or even open a pull request myself, if desired. Just let me know. And, above all, my apologies for the inconvenience to those whose instances are being impacted by this change.

Best regards,
Bryan Turner
Atlassian Bitbucket

[bturner]

I've opened #72 with a workaround for this issue.