fix(mysql): close consolidated acceptance review blockers#3162
Closed
weicao wants to merge 11 commits into
Closed
Conversation
…ent-only admin creds
mysql-myloader.sh connected with ${MYSQL_ADMIN_USER}/${MYSQL_ADMIN_PASSWORD},
but those are component-container env (credentialVarRef in _helpers.tpl) and are
not injected into the DataProtection restore job. The backup side
(mysql-mydumper.sh) already uses ${DP_DB_USER}/${DP_DB_PASSWORD}, and the same
myloader line already relies on the DP-injected ${DP_DB_HOST}/${DP_DB_PORT}, so
the user/password should come from the same DP connection env. With the admin
vars empty in the restore job, myloader fails authentication and the
mydumper/myloader logical restore fails.
Align the restore credentials with the backup side and add a shellspec that
asserts myloader is invoked with DP_DB_USER/DP_DB_PASSWORD and never references
MYSQL_ADMIN_*.
…shutdown proxysql-entry.sh backgrounded proxysql with `exec proxysql ... &` then `wait`, with no signal trap. On pod termination the wrapper (the container init process) received SIGTERM but never forwarded it to proxysql, so proxysql kept running until the termination grace period elapsed and was then SIGKILLed. This slows down / flaps scale-in, rolling-update, and delete tests. The mysql container already traps and forwards; proxysql should too (same class as #3090). - Install a `term_handler` trap on SIGTERM/SIGINT as the first thing in the main path (pid initialized to 0) so a signal during startup — config rendering, spawn, or the configure helper — is handled instead of the default kill. - The handler forwards TERM to the proxysql pid, reaps it exactly once, and exits, so the main path never runs a second `wait` (which would exit 127 "not a child" under set -e). - Drop the misleading `exec ... &` (exec cannot replace the shell for a backgrounded job) and the false "PID 1" comment; run proxysql as a child so the wrapper stays alive to forward signals. - Quote the FRONTEND_TLS_ENABLED test so an unset value is not a set -e syntax error. - Make the config helper / template / output paths overridable (in-image defaults unchanged) so the signal contract can be exercised end-to-end. Tests (scripts-ut-spec/proxysql_entry_spec.sh): - end-to-end: a real proxysql-stub child receives SIGTERM and the wrapper exits 0 (deterministic: waits until the child signals readiness before sending TERM); - double-wait guard: term_handler exits rather than falling through to a second wait (fails against a return-instead-of-exit handler); - trap installed before the proxysql spawn line; - term_handler forwards `kill -TERM <pid>` and is a no-op when no pid captured.
Two data-safety defects found in a deep review of the PITR binlog cleanup. 1. The fail-closed guard `synced_binlogs=$(get_synced_binlogs); if [ $? -ne 0 ]` is unreachable under the actionset-wide `set -e`: a separated `var=$(fn)` assignment where fn returns 1 aborts the whole script before the check, so a replica-parse failure crash-loops the archiver instead of logging and skipping the purge. (Splitting the earlier `local var=$(...)` to fix `$?` masking removed the mask that `set -e` was relying on.) Use `if ! synced_binlogs=$(get_synced_binlogs) || [ -z ... ]` so the failure is caught without tripping set -e. 2. `min_synced_file` was a leaked global, never reset. On a 2nd+ cleanup call in the long-lived archiver, if replica discovery returns empty the loop is skipped and the stale previous value passes the guard, purging binlogs on an unverified synced position (potential data loss). Declare it `local` with an empty reset at function entry. Both verified: bash confirms the separated assignment aborts under set -e while `if !` does not; helm render + bash -n pass.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## helios/mysql-review-consolidated-20260711 #3162 +/- ##
==========================================================================
Coverage 0.00% 0.00%
==========================================================================
Files 147 153 +6
Lines 23367 23680 +313
==========================================================================
- Misses 23367 23680 +313 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Independent composition review for Henry-owned M1/M2 scope at exact head Composition identity:
Contract checks:
Independent gates on this exact head:
XP design-contract classes 1-8 walked: class 1 no silent fallback; class 2 non-empty master/leaving-member identities enforced; class 3 no stale multi-read composition change; class 4 empty/invalid master sentinel checked fail-closed; classes 5-7 no cleanup/NotFound/release path in this scope; class 8 no unsafe mixed boolean precedence introduced. No M1/M2 composition blocker found. |
Contributor
Author
|
Closing this PR because its base is |
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
2 participants
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.
Base and scope
Exact base:
helios/mysql-review-consolidated-20260711@f163740e(main + 12-fix integration + #3117).This composition preserves owner commits as separate units and closes the deep-review findings before runtime acceptance:
set -e, resets per-call state, and matches uploaded files by live binlog prefix;innodb_redo_log_capacityis optional when memory is unknown;Local combined gate
Exact head:
a2a8796c.bash -n: PASSgit diff --check f163740e..HEAD: PASShelm dependency build addons/mysql: PASShelm lint addons/mysql: PASShelm template mysql addons/mysql: PASSRuntime boundary
Runtime N=0 on this combined exact head. This PR is not MySQL acceptance PASS, fullsuite PASS, or release readiness. Test execution must use the exact head and developer-specified focused scripts after this code/static gate is independently reviewed.