Skip to content

MDEV-17746: perfschema.dml_threads failed in buildbot with wrong errno#5395

Open
gkodinov wants to merge 1 commit into
10.11from
10.11-MDEV-17746
Open

MDEV-17746: perfschema.dml_threads failed in buildbot with wrong errno#5395
gkodinov wants to merge 1 commit into
10.11from
10.11-MDEV-17746

Conversation

@gkodinov

@gkodinov gkodinov commented Jul 16, 2026

Copy link
Copy Markdown
Member

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread mysql-test/suite/perfschema/t/dml_threads-master.opt Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 matching performance_schema.threads row exists before the error-expected update.
  • Increases performance_schema_max_thread_instances for the test via a new dml_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.

Comment thread mysql-test/suite/perfschema/t/dml_threads.test Outdated
Comment thread mysql-test/suite/perfschema/t/dml_threads-master.opt Outdated
@gkodinov
gkodinov force-pushed the 10.11-MDEV-17746 branch 2 times, most recently from 0196864 to 7597a99 Compare July 16, 2026 12:53
@vuvova

vuvova commented Jul 16, 2026

Copy link
Copy Markdown
Member

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.

@gkodinov

Copy link
Copy Markdown
Member Author

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.

@svoj

svoj commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

./mtr perfschema.dml_threads --repeat=100 fails reliably with the fix on the second iteration.

@svoj

svoj commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Simplified test:

--source include/have_perfschema.inc

select NAME, processlist_id from performance_schema.threads where processlist_id=connection_id();
--error ER_WRONG_PERFSCHEMA_USAGE
update performance_schema.threads
  set thread_id=12 where PROCESSLIST_ID=connection_id();

./mtr --repeat=2

select NAME, processlist_id from performance_schema.threads where processlist_id=connection_id();
NAME	processlist_id
thread/sql/one_connection	4
update performance_schema.threads
set thread_id=12 where PROCESSLIST_ID=connection_id();
ERROR HY000: Invalid performance_schema usage
main.AAA                                 [ pass ]      4
select NAME, processlist_id from performance_schema.threads where processlist_id=connection_id();
NAME	processlist_id
thread/sql/one_connection	8
update performance_schema.threads
set thread_id=12 where PROCESSLIST_ID=connection_id();
main.AAA                                 [ 2 fail ]
        Test ended at 2026-07-16 19:22:06

CURRENT_TEST: main.AAA
mysqltest: At line 5: query 'update performance_schema.threads
set thread_id=12 where PROCESSLIST_ID=connection_id()' succeeded - should have failed with error ER_WRONG_PERFSCHEMA_USAGE (1683)...

It demonstrates a valid row in the table whereas update still succeeds.

@gkodinov gkodinov added the MariaDB Foundation Pull requests created by MariaDB Foundation label Jul 17, 2026
@gkodinov
gkodinov force-pushed the 10.11-MDEV-17746 branch 2 times, most recently from de8cf04 to 69291a3 Compare July 17, 2026 11:53
@gkodinov

gkodinov commented Jul 17, 2026

Copy link
Copy Markdown
Member Author

Thanks, @svoj ! This was instrumental!

I've properly analyzed and debugged the issue today. And updated the pull request.
Now your example runs too with --repeat=100 if we increase the UPDATE value.

I also have a diff that might be interesting, but I'll leave that for Serg:

diff --git a/sql/sql_update.cc b/sql/sql_update.cc
index 891624f2f7e..49384df7fb5 100644
--- a/sql/sql_update.cc
+++ b/sql/sql_update.cc
@@ -58,7 +58,8 @@
 bool records_are_comparable(const TABLE *table) {
   return !table->versioned() &&
           (((table->file->ha_table_flags() & HA_PARTIAL_COLUMN_READ) == 0) ||
-           bitmap_is_subset(table->write_set, table->read_set));
+           bitmap_is_subset(table->write_set, table->read_set)) &&
+           table->file->ht->db_type != DB_TYPE_PERFORMANCE_SCHEMA;
 }

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.

@gkodinov
gkodinov requested review from svoj and vuvova July 17, 2026 11:57
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

@svoj svoj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();

@svoj svoj Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 startup
  • BIGINT_MAX - almost impossible to hit in current implementation
  • connection_id() - N - can be affected by lingering connections
  • connection_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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

MariaDB Foundation Pull requests created by MariaDB Foundation

Development

Successfully merging this pull request may close these issues.

4 participants