tests: stabilize TestUpdateMemberWhenRecovery

[okJiang]

What problem does this PR solve?

Issue Number: Close #9994

TestUpdateMemberWhenRecovery is flaky when all TSO nodes are stopped and one node is restarted.
The blocked GetTS request can stay attached to stale discovery or keyspace-group metadata long enough to hit its timeout even though the recovered TSO service is already able to serve fresh requests.

What is changed and how does it work?

Instead of waiting only on the original in-flight GetTS result, the test now treats recovery as successful when either:

• the blocked request finally returns a newer TS, or
• a fresh short-timeout GetTS retry returns a newer TS after the service recovers.

This keeps the original no-legacy-fallback assertion, but removes the timing dependency on when the first blocked request rebinds to refreshed TSO discovery.

Verification:

• cd tests/integrations && GOCACHE=/tmp/pd-gocache GOTMPDIR=/tmp/pd-gotmp make gotest GOTEST_ARGS='-tags=without_dashboard ./mcs/keyspace -run TestKeyspaceGroupTestSuite/TestUpdateMemberWhenRecovery -count=5 -v' -> PASS (5/5, ok ... 66.772s)
• GOCACHE=/tmp/pd-gocache GOTMPDIR=/tmp/pd-gotmp make basic-test -> FAIL in unrelated packages pkg/gctuner, pkg/schedule/operator, and pkg/storage/endpoint

tests: stabilize TestUpdateMemberWhenRecovery

Check List

Tests

• Integration test

Release note

None.

Summary by CodeRabbit

• Tests

  • Improved recovery validation by asserting a clear, monotonic timestamp increase after a service node restart, using asynchronous request handling and explicit cancellation to reduce flakiness.

• Refactor

  • Consolidated test retry logic into a single, reliable eventual-observation flow for recovery checks, simplifying control flow and improving stability.

[ti-chi-bot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ada9a4a8-8b38-40d7-b4b8-c22ae621f077

📥 Commits

Reviewing files that changed from the base of the PR and between a48b7a2192653413340d4dd8380d4b522ab31722 and 8b8fb94.

📒 Files selected for processing (1)

• tests/integrations/mcs/keyspace/tso_keyspace_group_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/integrations/mcs/keyspace/tso_keyspace_group_test.go

---

📝 Walkthrough

Walkthrough

Adjusts TestUpdateMemberWhenRecovery to detect TSO recovery by composing observed physical+logical timestamps: runs an async GetTS during a node restart, then repeatedly consumes in-flight or retried GetTS results to compute a recoveredTS and assert recoveredTS > initialTS, cancelling the original context afterward.

Changes

Cohort / File(s)	Summary
Test Recovery Logic tests/integrations/mcs/keyspace/tso_keyspace_group_test.go	Refactors TestUpdateMemberWhenRecovery: runs GetTS asynchronously during a TSO node restart, polls (consume in-flight result or retry GetTS) with short timeouts, composes recoveredTS via tsoutil.ComposeTS, asserts recoveredTS > initialTS, and cancels the original GetTS context after verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tests: fix TestTransferRegionWithPlacementRule and TestUpdateMemberWhenRecovery #10163 — Modifies the same TestUpdateMemberWhenRecovery and its GetTS concurrency/timeout handling.
• tests: stabilize TestUpdateMemberWhenRecovery retry path #10285 — Changes the test to tolerate transient GetTS failures by adding retry/verification logic after a TSO restart.
• tests: fix flaky TestUpdateDefaultReplicaConfig #10281 — Introduces eventual-observation patterns in the same test to handle transient GetTS errors.

Suggested reviewers

• JmPotato
• rleungx

Poem

🐰 I waited where timestamps sleep and pry,
A hiccup passed beneath the sky.
I stitched the bits — physical and small —
Recovered time rose past them all.
Hooray, said the rabbit, time hops high.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: stabilizing a flaky integration test by removing timing dependencies.
Description check	✅ Passed	The description fully covers the problem, solution, verification, and includes the required issue number, commit message, and test category.
Linked Issues check	✅ Passed	The PR directly addresses issue #9994 by implementing the objective to stabilize the flaky TestUpdateMemberWhenRecovery test through enhanced recovery detection logic.
Out of Scope Changes check	✅ Passed	All changes are limited to a single test file and directly target the flaky test mentioned in #9994; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.90%. Comparing base (033398b) to head (8b8fb94).
⚠️ Report is 21 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10442      +/-   ##
==========================================
+ Coverage   78.86%   78.90%   +0.04%     
==========================================
  Files         529      530       +1     
  Lines       71102    71548     +446     
==========================================
+ Hits        56072    56456     +384     
- Misses      11014    11072      +58     
- Partials     4016     4020       +4     

Flag	Coverage Δ
unittests	78.90% <ø> (+0.04%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[lhy1024]

The test goal has changed here: the original in-flight GetTS sent through resultCh no longer has to recover by itself; the test now passes as long as either that request or a fresh retry gets a newer TS after recovery.

I think the comments should be updated to match. Phrases like “will wait for TSO service to recover” and “Verify GetTS succeeds after node restart” still read more like they describe the original blocked request, which no longer matches the actual assertion.

[okJiang]

Updated the function header and the Step 5/6/7 comments to match the current assertion: we now describe eventual recovery, where either the in-flight request or a fresh retry can observe the recovered TSO service and return a newer TS. The wording no longer implies the original blocked request must recover by itself.

[okJiang]

Rechecked this thread: commit a48b7a219 updates the function header and the Step 5/6/7 comments to describe eventual recovery correctly, so the wording no longer implies the original in-flight request must recover by itself. The PR head is already at that commit.\n\n

[okJiang]

/retest

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lhy1024, rleungx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [lhy1024,rleungx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-03-20 10:10:31.273729377 +0000 UTC m=+524558.361386914: ☑️ agreed by lhy1024.
• 2026-04-01 05:55:19.954733295 +0000 UTC m=+330925.160093353: ☑️ agreed by rleungx.

[okJiang]

/retest

[okJiang]

/retest