Skip to content

Performance test: wait until the row counters have settled - #41

Merged
codeling merged 2 commits into
mainfrom
jarvis/affectionate-newton-8gwym4
Oct 25, 2026
Merged

codeling merged 2 commits into
mainfrom
jarvis/affectionate-newton-8gwym4

Conversation

@codeling

Copy link
Copy Markdown
Owner

Fixes a flaky failure of integration (PHP 8.3, MySQL 8.0) on #33. That PR's change (docker/login-action) isn't run by any test, and the same commit passed in its push run:

not ok 38 - creating an entry with an existing tag finds the tag by its index
  error: 'enter read 500 rows, at most 150 expected (table size 10000)'

Cause

The 500 rows aren't enter's own; locally it reads exactly 3, every time. They are the rows of SELECT COUNT(*) FROM todo WHERE deleted=1 AND list_id=10, which the empty-trash test runs right after its measurement (list 10 has 500 entries). rowsRead() waits until Threads_connected is 1, but MySQL can add a closed connection's counters to the global ones a moment after that. On the slower CI runner, the counters were read in that gap, so those 500 rows only showed up during the next measurement.

My first guess, that the overhead of reading the counters varies, was wrong: on MySQL 8.0.46 it is exactly 1000 rows in 40 of 40 samples. The first commit of this PR was based on that guess; the second one replaces it.

Change (tests/performance.test.js)

  • settledRowsRead(): before and after the request, the counters are read until two readings in a row differ by the overhead alone, so late counters are no longer counted for the request.
  • The overhead is the smallest difference of 3 pairs of readings, since late counters can only make a pair larger. Every reading after before adds one overhead, and readings counts them so each one is subtracted.
  • The limits are unchanged, so a full scan still fails the test.

Verified

  • MySQL 8.0.46 (the image CI uses), 3 runs: all 7 measurements give the same numbers as the old code (enter: 3, empty-trash: 502, …), no failures.
  • Simulated race (500 rows that show up only after the 3rd reading of the enter test): the old test fails with enter: 503 rows read, like in CI; the new one reports enter: 3. The real race didn't happen on my machine (0 in 60 tries), so this simulation is the evidence that it's fixed.
  • MariaDB, full npm test: 70/70 pass.

Generated by Claude Code

Reading the Handler_read_* counters reads rows itself on MySQL 8 (about 850),
and not always the same number. A single empty measurement could be some
hundred rows too low, so an index lookup was reported as 500 rows read
(limit 150): integration (PHP 8.3, MySQL 8.0) failed once on #33,
whose change no test ran, and passed on the same commit in the push run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016RquZy7Nysvr6gDdyjo4bc
The first fix assumed the overhead of reading the counters varies; on MySQL
8.0.46 it is exactly 1000 rows every time. The 500 rows were those of the
query in the empty-trash test after its measurement (list 10 has 500 entries):
MySQL can add a closed connection's counters a moment after
Threads_connected went down, and the slow CI runner read the counters in
that gap. Before and after the request, the counters are now read until two
readings in a row differ by the overhead alone; the overhead is the smallest
difference of a few pairs, and every reading's overhead is subtracted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016RquZy7Nysvr6gDdyjo4bc
@codeling
codeling merged commit 1c46112 into main Oct 25, 2026
24 checks passed
@codeling
codeling deleted the jarvis/affectionate-newton-8gwym4 branch October 25, 2026 08:00
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.

2 participants