Page MenuHomePhabricator

"Wait for Previous Commits to Build" step is really spammy
Closed, ResolvedPublic

Description

For build plans that have very long running time-frames (e.g. an hour+), using "Wait for Previous Commits to Build" to build commits incoming to a repository is ridiculously spammy.

Here's the rough test idea:

  • Have a repository with a build plan, that takes a long time to build. For example, simply sleep for like one hour.
    • The build plan should begin with 'Wait for Previous Commits to Build'
  • Push, say, 30 commits at once to the repository. All of the buildables will go into building status. One will actually be building, and the rest will be blockers. The builds will happen in order.
  • Go to the latest buildable for the latest commit - chronologically - i.e. the last patch that was pushed. This one will take the longest in terms of pure time, since it must wait on all previous builds.
  • The page containing build logs is gigantic. There are thousands of entries stating that a build is waiting on a previous build. This also adds a new 'build log' in its entirety and attaches it to the build itself.

In context: We have about 600 'Buildables' in Harbormaster. Each buildable only has 1 build inside, so there are about 600 overall builds too. In contrast, we have over 90,000 individual 'build logs' (probably corresponding to about 90,000 database rows), with 0 useful information. It's crazy.

Presumably blocked builders polled at a certain interval, to see if the blocker they're waiting on is done. And if this is not the case, then a build log with a message is attached to the build stating it is blocked.

Here's a full example:

https://phabricator.haskell.org/harbormaster/build/654/?l=100 (open registration, I'll fix permissions for build logs soon to be public).

Screenshot:

lotsofbuildlogsomg (1×1 px, 125 KB)

Note we are almost at build log #90,000, but only on build #654! And the page contains *thousands* of those empty build logs.

Event Timeline

thoughtpolice raised the priority of this task from to Normal.
thoughtpolice updated the task description. (Show Details)
thoughtpolice added a subscriber: thoughtpolice.
thoughtpolice renamed this task from Make "Wait for Previous Commits to Build" step considerably less spammy to "Wait for Previous Commits to Build" step is really spammy.Aug 28 2014, 3:31 PM

Oh, that is not supposed to be creating new logs for each entry. It's supposed to add them to the existing log.

What I think happened here is that at some point, that was converted to use yielding instead of a while (...) { ... sleep(5); } loop. Because it's yielding, it's now running the same behaviour (creation of logs) whenever it checks.

What needs to happen here is the ability to attach some sort of marker or data to PhabricatorWorkerYieldException so that build steps can continue where they left off.

Can you verify that D10383 fixes the issue?