Repository navigation
Performance test: wait until the row counters have settled - #41
Merged
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:Cause
The 500 rows aren't
enter's own; locally it reads exactly 3, every time. They are the rows ofSELECT 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 untilThreads_connectedis 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.beforeadds one overhead, andreadingscounts them so each one is subtracted.Verified
enter: 3,empty-trash: 502, …), no failures.entertest): the old test fails withenter: 503 rows read, like in CI; the new one reportsenter: 3. The real race didn't happen on my machine (0 in 60 tries), so this simulation is the evidence that it's fixed.npm test: 70/70 pass.Generated by Claude Code