Skip to content

fix(frontend): fence session reset from active requests - #26497

Open
VioletQwQ-0 wants to merge 1 commit into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-20022-proxy-reset-fence
Open

fix(frontend): fence session reset from active requests#26497
VioletQwQ-0 wants to merge 1 commit into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-20022-proxy-reset-fence

Conversation

@VioletQwQ-0

@VioletQwQ-0 VioletQwQ-0 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

issue #20022

What this PR does / why we need it:

Proxy can observe a terminal MySQL response and start ResetSession while the
original CN RoutineManager.Handler is still completing response accounting
and defers. The old lifecycle controller serialized reset/migration/close, but
did not serialize ordinary requests, so reset could close the old Session,
process, and transaction handler while Handler still owned them.

This change:

  • adds a lightweight request lease to the existing lifecycle controller;
  • makes ResetSession fail fast with the existing busy error while a request
    is active, so Proxy discards that backend and reconnects;
  • lets migration wait with its caller context, while routine close keeps its
    existing cancel-without-request-wait behavior;
  • rejects a new request before it reads Session if lifecycle work already owns
    the routine;
  • makes ExecRequest panic recovery safely handle a nil transaction handler.

Proxy cache/generation/in-flight production logic is unchanged.

Regression evidence

  • Base: 4959b7cdd853923f60b168d9ef69e242a1cadc37
    • TestRoutineManagerResetSessionRejectsRequestAfterResponseWrite failed:
      reset returned nil after the terminal response write while Handler was
      deliberately blocked before returning.
  • PR head: af17db6b3db4b19cf5b4a604395574ce21a4a602
    • the same deterministic interleaving returns busy, preserves the old
      Session/proc/TxnHandler and SessionManager entry, then succeeds once after
      Handler exits and serves the next COM_PING;
    • the race regression passed 100 consecutive iterations;
    • lifecycle-first, migration wait/cancel, close-no-wait, replacement reuse,
      rejected Proxy cache entry, and nil-TxnHandler recovery paths pass.

Validation

  • .agents/skills/mo-dev/scripts/mo-cgo-test ./pkg/frontend -count=1: PASS
  • .agents/skills/mo-dev/scripts/mo-cgo-test ./pkg/proxy -count=1: PASS
  • complete pkg/frontend and pkg/proxy with -race -count=1: PASS
  • focused request/reset interleaving with -race -count=100: PASS
  • go build -mod=readonly ./pkg/frontend ./pkg/proxy: PASS
  • go vet -mod=readonly ./pkg/frontend ./pkg/proxy: PASS
  • git diff --check: PASS
  • schema-v4 preflight: PASS review=PASS validation=PENDING
  • current exact-head required CI: PASS, including Ubuntu UT, SCA, UT coverage,
    Proxy Compose BVT, pessimistic launch BVT, and coverage.

Local deployment evidence

The local topology was launched with the real Proxy path
(mo-service -with-proxy), 2 CNs, and true new client connections.

  • direct-CN control, 100 workers / 2 s / 2 min: 5,987 queries, 0 failures;
  • Proxy cache disabled, 100 workers / 2 s / 2 min: 5,988 queries and one
    isolated 5 s context deadline; all Proxy/CN probes passed;
  • Proxy cache enabled after warm-up, 100 workers / 1 s / 60 s: 5,995 queries,
    0 failures, all 36 endpoint probes passed, and backend sessions converged
    from 119 to 17/18 rather than growing with total connections;
  • no CN/Proxy panic, pod/process restart, wait active, or
    createTxnOpUnsafe was observed in these valid runs.

A malformed first result was observed in two cold-start runs immediately after
ports opened. A later diagnostic run without s2c batching passed 5,984 queries,
but it ran after the cluster was fully warm and therefore does not prove a
buffering defect. This cold-start observation is tracked as unresolved
acceptance evidence and is not used to justify an unrelated Proxy code change.

The complete 10-minute local soak is still pending: the initial 100
connections/s setup exhausts the macOS loopback ephemeral-port range, while
the later 50 connections/s rerun was interrupted by the local disk safety
threshold. TKE validation remains pending.

QA / BVT

  • QA required: yes
  • BVT: N/A — regular SQL BVT cannot enable Proxy connection cache or create
    independent true short connections. Deterministic Frontend/Proxy concurrency
    tests provide the in-repo regression coverage.
  • Deployment validation pending:
    • local 2 CN + Proxy, warmed and readiness-gated, 100 concurrent true short
      connections for 10 minutes at a host-safe connection rate;
    • TKE 3 CN + 2 Proxy, 1000 vusers for 10 minutes;
    • TKE 300 terminals for 70 minutes, with independent Proxy/CN login probes,
      no panic/restart/continuous login failure, and bounded backend sessions.
  • After merge, keep [Bug]: config proxy cache connection mo hung #20022 open under phase/testing for Ariznawlll to run
    the deployment matrix; link matrixorigin/mo-nightly-regression#835 for the
    recurring short-connection coverage.

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

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

Labels

kind/bug Something isn't working size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants