fix(mysql): make PITR disk-pressure binlog cleanup functional and prefix-safe#3088
Draft
weicao wants to merge 4 commits into
Draft
fix(mysql): make PITR disk-pressure binlog cleanup functional and prefix-safe#3088weicao wants to merge 4 commits into
weicao wants to merge 4 commits into
Conversation
…fix-safe Replica discovery relied on KB_ITS_*_HOSTNAME env that never existed in DataProtection job pods on KB 1.x, so the >=80% disk cleanup (default PURGE_BINLOG=on) silently never purged anything. Discover replicas from the target primary via SHOW REPLICAS / SHOW SLAVE HOSTS instead. Also make the master purge prefix-safe: PURGE BINARY LOGS TO is prefix deletion, so scan stops at the first file that must be kept (not yet synced+uploaded, or one of the newest 5) and a single PURGE TO that file is issued - the old per-file loop could delete an unarchived binlog sitting before a qualifying one and tear the archived sequence. Membership tests are now exact-match instead of substring grep. Fixes #3087 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3088 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 144 144
Lines 23013 23013
=====================================
Misses 23013 23013 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The disk-pressure binlog purge only triggers at >=80% usage, which cannot be safely reached on a large shared host volume in tests (would need a ~177GB fill). Make the threshold overridable via PURGE_BINLOG_DISK_THRESHOLD (default 80, production behaviour unchanged) so a test can set it below the current usage and exercise the real purge path without filling the disk. Refs #3087 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
get_synced_binlogs matched replica status against a reconstructed
${DP_TARGET_POD_NAME}-bin, which fails when the target env drifts from
the actual binlog-writing pod (e.g. after a switchover) - returning
'No synced binlog files found' even when replicas are synced. Match
against LOG_PREFIX (derived from the real log_bin_basename) instead.
Add DEBUG lines printing DP_TARGET_POD_NAME/LOG_PREFIX/replica hosts and
per-replica resolved current_file, so the cleanup-decision path is
observable in the archive-binlog log.
Refs #3087
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…stderr
get_replica_hosts assumed replicas register report_host, but the semisync
cmpd start command omits --report-host (only the MGR cmpd and
mysql-entrypoint.sh set it). With report_host empty, SHOW REPLICAS leaves the
Host column blank and `awk '{print $2}'` slides onto the Port, returning
"3306" as the replica host (observed: 'Processing replica host: 3306',
'Can't connect to MySQL server on 3306:3306'). Discover connected replicas
from the primary's own Binlog Dump threads instead
(information_schema.PROCESSLIST), which carry the replica ip:port regardless
of report_host; strip the ephemeral :port. Fall back to SHOW REPLICAS / SHOW
SLAVE HOSTS with TAB-split awk so an empty Host stays empty instead of
becoming the Port.
Also move the two DEBUG lines in get_synced_binlogs to stderr: that
function's stdout is captured by `synced_binlogs=$(get_synced_binlogs)`, so
echoing on stdout both hid the lines from the pod log and corrupted the
return value (making the empty result look non-empty and bypassing the
fail-closed 'No synced binlog files found' guard).
Split the `local synced_binlogs=$(...)` declaration from its assignment so
the `$? -ne 0` check reflects get_synced_binlogs' exit code rather than the
always-zero exit of `local`, keeping the fail-closed guard closed.
Refs #3087
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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 #3087.
Three changes, all inside
cleanup_mysql_binlogs:get_replica_hostsqueries the target primary (SHOW REPLICAS, falling back toSHOW SLAVE HOSTSfor 5.7) — replicas register their report_host there. Replaces the deadKB_ITS_*_HOSTNAMEenv scan (both call sites).PURGE BINARY LOGS TOdeletes everything before the named file, so the scan stops at the first file that must be kept (not synced+uploaded, or within the newest 5 — the old code actually kept 4) and issues onePURGE TOfor the safe prefix. The old per-file loop could delete an unarchived binlog and break PITR continuity.grep -Fxqover newline lists) instead of substringgrep -q.Conservative boundary kept: if no replica can be confirmed (query failure or zero registered replicas), master purge is still skipped, same as before.
Validation:
bash -npass. Runtime matrix for the mysql team via kubeblocks-tests before undraft: multi-replica cluster at >=80% disk — purge executes and keeps unarchived+newest files; replica lagging — purge stops before its sync point; orchestrator/replica down — cleanup skips; 5.7 path uses SHOW SLAVE HOSTS.🤖 Generated with Claude Code