MDEV-39006: Galera test failure on galera_3nodes.galera_garbd_backup by hemantdangi-gc · Pull Request #5018 · MariaDB/server

hemantdangi-gc · 2026-04-29T08:41:42Z

Issue:
sst_disable_innodb_writes() promises no writes to persistent files between Galera donor state and SST release, and most engine subsystems already honour that (purge, dict_stats, encryption thread count, FTS optimizer in fts0opt.cc:2844). The page cleaner did not: buf_flush_page_cleaner()'s set_almost_idle: block ran buf_dblwr.flush_buffered_writes() and log_checkpoint() unconditionally on every wake-up. With LSN advanced past last_checkpoint_lsn (via any prior background mtr - the most common one being a wsrep applier writeset), log_checkpoint_low() then appended a fresh FILE_CHECKPOINT record to ib_logfile0 mid-SST, breaking
galera_3nodes.galera_garbd_backup with a --diff_files mismatch.

Solution:
Add a wsrep_sst_disable_writes gate around those two writes, mirroring the FTS pattern. In non-WSREP builds the flag is a constexpr false and the check compiles away. This closes the page-cleaner leak; other writers (wsrep apply paths, in-flight encryption rotation, ib_buffer_pool dump) are separate concerns and out of scope.

Also assert !recv_no_log_write after log_write_up_to() in log_checkpoint_low() so debug builds catch any future write path that slips through.

dr-m · 2026-04-29T12:29:08Z

+  DBUG_EXECUTE_IF("sst_force_lsn_advance",
+  {
+    /* Test-only: write a single FILE_MODIFY redo record so that
+    log_sys.get_lsn() advances past
+      last_checkpoint_lsn + SIZE_OF_FILE_CHECKPOINT + 8*is_encrypted().
+    A subsequent buf_flush_page_cleaner() wake-up will then enter
+    log_checkpoint_low() past its early-return and, absent the
+    wsrep_sst_disable_writes gate in buf0flu.cc, fire the
+    recv_no_log_write tripwire (debug) or write a FILE_CHECKPOINT
+    record to ib_logfile0 (release). With the
+    UNIV_LIKELY(!wsrep_sst_disable_writes) check in place this
+    extra LSN gets checkpointed past once sst_enable_innodb_writes()
+    runs, so it does not survive past the SST window). */
+    skip_verify_checkpoint= true;
+    log_sys.latch.wr_lock();
+    mtr_t mtr;
+    mtr.start();
+    mtr.log_file_op(FILE_MODIFY, SRV_SPACE_ID_UPPER_BOUND - 1,
+                    "./mdev39006_fake.ibd");
+    const lsn_t lsn_after_modify= mtr.commit_files();
+    log_sys.latch.wr_unlock();
+    log_write_up_to(lsn_after_modify ? lsn_after_modify : log_sys.get_lsn(),
+                    true);
+  });


This causes a compilation failure on many debug builders, because the low-level member function mtr_t::log_file_op() is not intended to be invoked directly, but only via other functions, such as mtr_t::name_write().

Is there really no other way to trigger this scenario? Could we write a dummy FILE_CHECKPOINT record instead, like innodb_log_file_size_update() does?

missed that as it run successfully on local.
Thanks for checking issue, I added a dummy function to resolve the issue as mtr_t::name_write() and innodb_log_file_size_update() needed some more changes.

So this piece of code can be removed?

janlindstrom

I have some improvement requests.

janlindstrom · 2026-04-30T11:09:37Z

+# (debug) or writes FILE_CHECKPOINT to ib_logfile0 (release).
+SET GLOBAL innodb_max_dirty_pages_pct_lwm=0.0;
+SET GLOBAL innodb_max_dirty_pages_pct=0.0;
+--sleep 2


Is there any other way to see that page cleaner had a change to run? This introduces a race, page cleaner thread might not get change to run during this 2 second wait leading to fact that this test is not testing intended behaviour.

The page cleaner is a background thread spawned via std::thread, with no THD and no current_thd, it cannot host a per-THD DEBUG_SYNC action. We can increase sleep time to 5 second.

There is no counter available for dirty_pages that we could query?

There's no existing status variable that increments per page-cleaner idle iteration. Innodb_buffer_pool_pages_dirty is already 0 throughout the SST window and never changes; pages_flushed, dblwr_writes, MONITOR_FLUSH_BACKGROUND_COUNT only increment on actual flush work, which the cleaner's set_almost_idle: block doesn't do.

The cleaner does flip buf_pool.page_cleaner_idle() right before reaching set_almost_idle:. That predicate is exactly what we'd want to observe - it goes false on page_cleaner_wakeup() and true again once the cleaner has fully completed an iteration. But it's not exposed as a status variable, sysvar, or INFORMATION_SCHEMA row anywhere. Adding it as a status variable is the cleanest fix.

@hemantdangi-gc I do not think you need any new test cases and any of this debug injection. Current test cases already cover code path effected. See e.g. https://es-jenkins.mariadb.net/job/Test-Package//725056/artifact/test-MTR-custom.log/*view*

I agree that current test covers issue and new test which is just forced injection can be removed.

But from where you got https://es-jenkins.mariadb.net/job/Test-Package//725056/artifact/test-MTR-custom.log/*view. Is this link from MDEV-3006 jenkins run? The link you provided is build on revision: 363280c179e6c8f7a62405b69121c276b7b3dfe3 but the current patch is on revision: "7106d8d72144117f71ef5feac56374207aab0de9".

janlindstrom · 2026-04-30T11:13:54Z

+# recv_no_log_write and resumes normal page-cleaner operation.
+# (raise pct first so the lwm setter does not warn about lwm > pct.)
+SET GLOBAL innodb_max_dirty_pages_pct=99;
+SET GLOBAL debug_dbug = "+d,sst_enable_writes_now";


Should we clear dbug_dbug before setting max_dirty_pages* values?

janlindstrom · 2026-04-30T11:17:31Z

+  DBUG_EXECUTE_IF("sst_force_lsn_advance",
+  {
+    /* Test-only: write a single FILE_MODIFY redo record so that
+    log_sys.get_lsn() advances past
+      last_checkpoint_lsn + SIZE_OF_FILE_CHECKPOINT + 8*is_encrypted().
+    A subsequent buf_flush_page_cleaner() wake-up will then enter
+    log_checkpoint_low() past its early-return and, absent the
+    wsrep_sst_disable_writes gate in buf0flu.cc, fire the
+    recv_no_log_write tripwire (debug) or write a FILE_CHECKPOINT
+    record to ib_logfile0 (release). With the
+    UNIV_LIKELY(!wsrep_sst_disable_writes) check in place this
+    extra LSN gets checkpointed past once sst_enable_innodb_writes()
+    runs, so it does not survive past the SST window). */
+    skip_verify_checkpoint= true;
+    log_sys.latch.wr_lock();
+    mtr_t mtr;
+    mtr.start();
+    mtr.log_file_op(FILE_MODIFY, SRV_SPACE_ID_UPPER_BOUND - 1,
+                    "./mdev39006_fake.ibd");
+    const lsn_t lsn_after_modify= mtr.commit_files();
+    log_sys.latch.wr_unlock();
+    log_write_up_to(lsn_after_modify ? lsn_after_modify : log_sys.get_lsn(),
+                    true);
+  });


So this piece of code can be removed?

hemantdangi-gc · 2026-05-04T07:10:52Z

So this piece of code can be removed?

No removing the sst_force_lsn_advance block would render the test useless. The injection is a one-byte LSN bump that takes us off that exact-match boundary, mirroring what real-world background activity (a wsrep applier writeset, encryption page rotation) intermittently does on the donor and which made the original galera_3nodes.galera_garbd_backup test flaky. It lives behind DBUG_EXECUTE_IF so it's a no-op in non-debug builds and only fires when the MDEV-39006 test arms it.

Issue: sst_disable_innodb_writes() promises no writes to persistent files between Galera donor state and SST release, and most engine subsystems already honour that (purge, dict_stats, encryption thread count, FTS optimizer in fts0opt.cc:2844). The page cleaner did not: buf_flush_page_cleaner()'s set_almost_idle: block ran buf_dblwr.flush_buffered_writes() and log_checkpoint() unconditionally on every wake-up. With LSN advanced past last_checkpoint_lsn (via any prior background mtr - the most common one being a wsrep applier writeset), log_checkpoint_low() then appended a fresh FILE_CHECKPOINT record to ib_logfile0 mid-SST, breaking galera_3nodes.galera_garbd_backup with a --diff_files mismatch. Solution: Add a wsrep_sst_disable_writes gate around those two writes, mirroring the FTS pattern. In non-WSREP builds the flag is a constexpr false and the check compiles away. This closes the page-cleaner leak; other writers (wsrep apply paths, in-flight encryption rotation, ib_buffer_pool dump) are separate concerns and out of scope. Also assert !recv_no_log_write after log_write_up_to() in log_checkpoint_low() so debug builds catch any future write path that slips through.

janlindstrom

Please remove unnecessary debug-injection it is totally unnecessary as existing test suite covers effected code.

janlindstrom · 2026-05-06T05:10:39Z

+# (debug) or writes FILE_CHECKPOINT to ib_logfile0 (release).
+SET GLOBAL innodb_max_dirty_pages_pct_lwm=0.0;
+SET GLOBAL innodb_max_dirty_pages_pct=0.0;
+--sleep 2


@hemantdangi-gc I do not think you need any new test cases and any of this debug injection. Current test cases already cover code path effected. See e.g. https://es-jenkins.mariadb.net/job/Test-Package//725056/artifact/test-MTR-custom.log/*view*

hemantdangi-gc force-pushed the 10.11-MDEV-39006 branch 2 times, most recently from 6824491 to 39d9068 Compare April 29, 2026 10:54

dr-m requested changes Apr 29, 2026

View reviewed changes

hemantdangi-gc force-pushed the 10.11-MDEV-39006 branch from 39d9068 to c9e308c Compare April 30, 2026 05:20

hemantdangi-gc requested review from dr-m and janlindstrom April 30, 2026 06:37

gkodinov added the MariaDB Corporation label Apr 30, 2026

janlindstrom requested changes Apr 30, 2026

View reviewed changes

hemantdangi-gc force-pushed the 10.11-MDEV-39006 branch from c9e308c to 7106d8d Compare May 4, 2026 07:16

janlindstrom requested changes May 6, 2026

View reviewed changes

Uh oh!

Conversation

hemantdangi-gc commented Apr 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janlindstrom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hemantdangi-gc commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janlindstrom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

hemantdangi-gc commented May 4, 2026 •

edited

Loading