Skip to content

TD-2600: separate browser liveness from test progress in the server watchdog - #174

Open
wmech-tiugo wants to merge 1 commit into
masterfrom
feature/TD-2600-watchdog-liveness
Open

wmech-tiugo wants to merge 1 commit into
masterfrom
feature/TD-2600-watchdog-liveness

Conversation

@wmech-tiugo

Copy link
Copy Markdown

TL;DR

The server judged a remote browser by a single 55 second budget that answered two
different questions at once: "has the browser stopped responding?" and "has it
finished a test recently?". Because results are batched client-side every 30
seconds and a chunk reload posts nothing at all, a browser that was merely slow
looked exactly like a dead driver, and the whole lane was killed.

This splits the two. Silence from the browser is still caught after 55 seconds.
A browser that is responding but slow now gets --progressTimeout, default 5
minutes. The runner sends keep-alive pings in auto mode as well as manual, so
the server has evidence of liveness other than test results.

One budget answering two questions

session.updated is bumped by exactly three routes: /tests/start,
/tests/results and /tests/done (Apis.ts). /tests/alive only sets
session.alive, and in auto mode the runner never sent it at all, because the
keep-alive timer was guarded by if (harness.mode === 'manual').

So the watchdog was fed only by posted test results, and Reporter flushes those
at most every 30 seconds. The real budget was therefore closer to 25 seconds than
55, and a chunk boundary spends it on a full page reload that re-fetches the whole
test bundle while posting nothing.

This is the second half of TINYMCE-14879, which fixed the start-ping side of the
same problem: "if there is a test-fail-reload or a chunk boundary reload, updates
are not sent to the server".

Two clocks instead of one

Controller.awaitDone now checks them separately:

  • Liveness - now - session.alive > 55s. Every route bumps session.alive,
    so this fires only when nothing at all is getting through. The message is
    unchanged, No updates from the browser in Xs, assuming driver has failed.
  • Progress - now - session.updated > progressTimeout. New message, and a new
    --progressTimeout CLI option defaulting to 5 minutes.

The default sits below the 360 second idle timeout we already advertise to the
remote grid (REMOTE_IDLE_TIMEOUT_SECONDS), so bedrock still decides when to give
up rather than the grid.

Why auto mode needs the keep-alive back

Splitting the clocks is only useful if something feeds the liveness one. In auto
mode nothing did, so the runner now sends the ping in both modes.

That exposed a second problem: /tests/alive called keepAliveAction() with no
throttle, while the runner pings every 5 seconds. Enabling it in auto mode would
have issued a webdriver command every 5 seconds on every lane. The route now uses
maybeSendKeepAlive(), the throttled variant already used by /tests/results.

If an older client never pings, liveness and progress collapse back to the same
clock and behaviour is exactly what it is today.

Verified

yarn test green: lint, build of all four projects, and 176 tests passing
(26 + 3 + 92 + 55).

ClisTest asserts the whole resolved settings object, so the new option is added
to its nine expectation blocks. No other test needed changing.

Not yet exercised against a real remote grid. The behaviour worth confirming there
is that a slow lane now finishes slowly instead of being killed, which is the whole
point of the change.

Why now

On 24-25 September a LambdaTest slowdown cost 38 builds a browser session, 59.4%
of every failure in that window, and tinymce-premium finished a day at 1 green
against 31 red with zero failing tests, because nothing ran far enough for a test
to fail.

Nothing on our side changed: build 40 passed and build 41 failed on the identical
commit, config, bedrock version and tunnel client. Across 14 of those kills the
server reported 55.133s to 55.973s against its own 55 second threshold, so it was
fully responsive at the moment it gave up. These were healthy sessions killed on a
budget that had run out, not hung ones.

See TD-2600.

What the reviewer should check

  • The 5 minute default. It is a judgement call. Lower is safer for a genuinely
    stuck run, higher is kinder to a slow grid. It only applies while the browser is
    still responding.
  • The ping interval against the liveness budget. The runner pings every 5
    seconds against a 55 second budget, so ten consecutive pings have to be lost
    before a healthy lane dies.
  • The new progress message. Our CI failure classifier keys on these strings.
    The liveness message is deliberately unchanged; the progress one is new and will
    need a companion rule in jenkins-build-analytics.

…atchdog

The server used a single 55 second budget for both "the browser has stopped
responding" and "the browser has not finished a test", so a remote browser that
was merely slow was killed as if its driver had died.

- Controller now tracks liveness (session.alive) separately from progress
  (session.updated). Silence from the browser is still caught after 55 seconds;
  a browser that is responding but slow gets the new --progressTimeout, which
  defaults to 5 minutes.
- The runner sends keep-alive pings in auto mode as well as manual. Results are
  batched and a page reload posts nothing at all, so without them the server has
  no evidence of liveness other than test results.
- /tests/alive now uses the throttled maybeSendKeepAlive, so the extra pings do
  not issue a webdriver command every few seconds.
@wmech-tiugo
wmech-tiugo requested a review from a team as a code owner September 25, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant