Bug 221943 - [perf.webkit.org] Do not schedule jobs to Buildbot if the last job failed
Summary: [perf.webkit.org] Do not schedule jobs to Buildbot if the last job failed
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Johnson
Keywords: InRadar
Depends on:
Reported: 2021-02-15 17:54 PST by Dean Johnson
Modified: 2021-02-16 18:28 PST (History)
4 users (show)

See Also:

v1 (12.88 KB, patch)
2021-02-15 18:05 PST, Dean Johnson
no flags Details | Formatted Diff | Diff
v2 (12.89 KB, patch)
2021-02-16 09:52 PST, Dean Johnson
dewei_zhu: review+
Details | Formatted Diff | Diff
v3 (13.03 KB, patch)
2021-02-16 17:31 PST, Dean Johnson
no flags Details | Formatted Diff | Diff
v3-1 (13.03 KB, patch)
2021-02-16 17:32 PST, Dean Johnson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Johnson 2021-02-15 17:54:28 PST
The syncing scripts for A/B infrastructure currently only require that there are no pending builds and no running test groups assigned to a builder before a new test group is assigned to it. This means that if a builder becomes available, then runs a test that puts the host into a state where it fails until someone manually fixes it, the broken host can fail all pending jobs that match the failing configuration.

To address this, we will start running periodic health checks against the A/B queues for a buildmaster and only schedule jobs when the last build (or health check) succeeded and previously noted conditions are true.
Comment 1 Dean Johnson 2021-02-15 17:59:04 PST
Comment 2 Dean Johnson 2021-02-15 18:05:54 PST
Created attachment 420408 [details]
Comment 3 Dean Johnson 2021-02-16 09:52:39 PST
Created attachment 420482 [details]

Fixes style checker errors.
Comment 4 dewei_zhu 2021-02-16 15:37:39 PST
Comment on attachment 420482 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=420482&action=review

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:84
> +    lastCompletedBuildSuccessful() { return this._lastCompletedBuild == null || this._lastCompletedBuild.result() == 0; }

Ditto per https://webkit.org/code-style-guidelines/#zero-comparison

> Websites/perf.webkit.org/tools/js/buildbot-syncer.js:193
> +                    if ((!this._lastCompletedBuild || this._lastCompletedBuild.buildTag() < entry.buildTag()) && entry.hasFinished())

buildTag doesn't guarantee to be indicative of build time, but in Buildbot case, the larger build tag means more recent

> Websites/perf.webkit.org/tools/js/buildbot-triggerable.js:254
> +            if (syncer.isTester() && nextRequest.order() == 0 && !syncer.lastCompletedBuildSuccessful())

Nit: per webkit code style guideline https://webkit.org/code-style-guidelines/#zero-comparison, it should be `!nextRequest.order()`
Also leave a comment about currently limiting to tester syncer.
Comment 5 dewei_zhu 2021-02-16 15:37:49 PST
r=me with comments.
Comment 6 Dean Johnson 2021-02-16 17:31:14 PST
Created attachment 420563 [details]

Fixed style issues.
Comment 7 Dean Johnson 2021-02-16 17:32:19 PST
Created attachment 420564 [details]
Comment 8 EWS 2021-02-16 18:27:31 PST
Committed r272971: <https://commits.webkit.org/r272971>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420564 [details].
Comment 9 Radar WebKit Bug Importer 2021-02-16 18:28:17 PST