MDEV-17746: perfschema.dml_threads failed in buildbot with wrong errno#5395
MDEV-17746: perfschema.dml_threads failed in buildbot with wrong errno#5395gkodinov wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the dml_threads performance schema test to query the current connection's thread ID before attempting an invalid update, ensuring the target row exists. It also introduces a master option file setting performance_schema_max_thread_instances to 50,000. The reviewer pointed out that this value is higher than the 10,000 specified in the pull request description, which could cause unnecessary memory consumption, and recommended lowering it to 10,000 and adding a trailing newline.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
3ea6257 to
da86bb0
Compare
There was a problem hiding this comment.
Pull request overview
Stabilizes the perfschema.dml_threads MTR test under heavy concurrency by ensuring the session’s performance_schema.threads row exists before running an UPDATE ... WHERE PROCESSLIST_ID=CONNECTION_ID() that is expected to error, and by increasing the PFS thread-instance sizing for this test run.
Changes:
- Adds a deterministic
SELECT(with--replace_column) to confirm a matchingperformance_schema.threadsrow exists before the error-expected update. - Increases
performance_schema_max_thread_instancesfor the test via a newdml_threads-master.opt. - Updates the expected result output accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| mysql-test/suite/perfschema/t/dml_threads.test | Adds a pre-check SELECT (with column replacement) before the error-expected update. |
| mysql-test/suite/perfschema/t/dml_threads-master.opt | Sets a higher performance_schema_max_thread_instances for this test to reduce flakiness under load. |
| mysql-test/suite/perfschema/r/dml_threads.result | Records the new SELECT output in the expected result. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0196864 to
7597a99
Compare
|
Looks strange. What do you mean "can happen under heavy concurrent load" ? There is no concurrent load in mtr, every test always runs on a dedicated server, "concurrent" mtr means many servers running concurrently, not all tests on a single server. |
|
Hitting that limit is the only possible explanation on why this particular query fails to update and the one prior to it is passing. What causes the thread depletion is a subject to a whole different study. But this bug is about stability of the test. |
|
|
|
Simplified test: ./mtr --repeat=2 It demonstrates a valid row in the table whereas update still succeeds. |
de8cf04 to
69291a3
Compare
|
Thanks, @svoj ! This was instrumental! I've properly analyzed and debugged the issue today. And updated the pull request. I also have a diff that might be interesting, but I'll leave that for Serg: With this diff, even the original 12 is working. I find it a bit too intrusive, but if it's to your liking I can push that too. |
The test was trying to update the performance_schema.threads row for the current connection. And if that row was not found or was the same, there's nothing to update hence no update not allowed error. MariaDB has a device to skip calling ha_update_row() if the old and the new records are identical. The connection id is increasing by at least 2 for every test run. Eventually it will hit the constant 12 in the UPDATE and the server will not call ha_update_row() at all and won't get the error. This is not a bug: it's reasonable behavior. Stablizied the test by changing the UPDATE value to 100k. In this way we can repeat the test at least 50k times before it breaks. Co-authored-by: Grok
69291a3 to
0bb7128
Compare
svoj
left a comment
There was a problem hiding this comment.
Nice. The change looks good, I don't see the need to update the code. Just one suggestion re THREAD_ID value.
| --error ER_WRONG_PERFSCHEMA_USAGE | ||
| update performance_schema.threads | ||
| set thread_id=12 where PROCESSLIST_ID=connection_id(); | ||
| set thread_id=100000 where PROCESSLIST_ID=connection_id(); |
There was a problem hiding this comment.
100000 might be too close to what we could reasonably get using mtr. I'd use some other value:
0- IIUC PFS still may allocate this value to some thread at early startupBIGINT_MAX- almost impossible to hit in current implementationconnection_id() - N- can be affected by lingering connectionsconnection_id() + N- sounds reliable assuming the default connection is the most recent one.
UPD: argh, connection_id() is from a different namespace, but you got the idea.
The test was trying to update the performance_schema.threads row for
the current connection. And if that row was not found or was the same,
there's nothing to update hence no update not allowed error.
MariaDB has a device to skip calling ha_update_row() if the old and the
new records are identical.
The connection id is increasing by at least 2 for every test run.
Eventually it will hit the constant 12 in the UPDATE and the server
will not call ha_update_row() at all and won't get the error.
This is not a bug: it's reasonable behavior.
Stablizied the test by changing the UPDATE value to 100k.
In this way we can repeat the test at least 50k times before it breaks.