test: fix flaky test TestForwardTestSuite in next-gen #10203

okJiang · 2026-02-03T07:20:41Z

What problem does this PR solve?

Issue Number: Close #10200

What is changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Code changes

Has the configuration change
Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tiup:
Need to cherry-pick to the release branch

Release note

None.

Summary by CodeRabbit

Bug Fixes
- Improved startup cleanup to reliably stop embedded services, close clients and network connections on startup failures, reducing resource leaks.
Tests
- Hardened cluster tests to treat cluster ID mismatches like port conflicts, automatically recreate servers with new ports and retry to improve test reliability.

Signed-off-by: okjiang <819421878@qq.com>

coderabbitai · 2026-02-03T07:21:04Z

📝 Walkthrough

Walkthrough

Introduces a startup cleanup routine in server/server.go that ensures embedded etcd and related clients/connections are closed on start failures, and updates tests/cluster.go to treat "Etcd cluster ID mismatch" as a retriable port-conflict, recreating servers and retrying startup.

Changes

Cohort / File(s)	Summary
Startup resource cleanup `server/server.go`	Changed `startEtcd` signature to return a named error (`retErr`) and added a deferred `cleanup()` closure that stops the embedded etcd, closes etcd/election clients, and closes HTTP idle connections on error paths.
Test retry for cluster ID mismatch `tests/cluster.go`	Expanded `runInitialServersWithRetry` to treat "Etcd cluster ID mismatch" like "address already in use": stop/destroy servers, recreate ports, sleep briefly, and retry startup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

tests: retry when address already in use #10162: Similar edits to tests/cluster.go extending retry handling to additional etcd startup errors (port-conflict flow).

Suggested labels

size/L, lgtm, approved

Suggested reviewers

bufferflies
lhy1024
JmPotato

Poem

🐰 I hopped through logs at break of day,
I closed the streams when starts went astray,
If IDs clash or ports misbehave,
I'll remake the nest and bravely try to save. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the fix for the flaky TestForwardTestSuite test identified in issue `#10200`.
Description check	✅ Passed	The description includes the linked issue (`#10200`) and mostly follows the repository template, though implementation details are minimal.
Linked Issues check	✅ Passed	The code changes address the root cause of the TestForwardTestSuite flakiness by improving etcd cleanup on errors and handling cluster ID mismatches as port conflicts requiring retry.
Out of Scope Changes check	✅ Passed	All changes in server/server.go and tests/cluster.go are directly related to fixing the flaky test by improving error handling and cleanup.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

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

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

okJiang · 2026-02-03T12:59:26Z

/retest

Signed-off-by: okjiang <819421878@qq.com>

codecov · 2026-02-04T11:34:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.64%. Comparing base (2139230) to head (80ce0e5).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10203      +/-   ##
==========================================
+ Coverage   78.63%   78.64%   +0.01%     
==========================================
  Files         520      520              
  Lines       70089    70104      +15     
==========================================
+ Hits        55112    55134      +22     
+ Misses      10989    10988       -1     
+ Partials     3988     3982       -6

Flag	Coverage Δ
unittests	`78.64% <100.00%> (+0.01%)`	⬆️

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.

okJiang · 2026-02-04T15:41:00Z

/retest

okJiang · 2026-02-05T03:07:24Z

/retest

JmPotato · 2026-02-06T09:04:39Z

server/server.go

+		// NOTE: `embed.Etcd.Close()` can block for a long time in some failure paths
+		// (e.g. when starting a removed member that can never become ready). Avoid
+		// blocking the caller (tests may wait for the start error) by stopping the
+		// server synchronously and closing the embedded etcd in background.


You mean "asynchronously"?

JmPotato · 2026-02-06T09:05:51Z

server/server.go

 	}
 	err = s.initMember(newCtx, etcd)
 	if err != nil {
+		cleanup()


What about using a defer that checks whether err is nil to decide whether to call cleanup()?

Signed-off-by: okjiang <819421878@qq.com>

ti-chi-bot · 2026-02-10T05:31:44Z

[LGTM Timeline notifier]

Timeline:

2026-02-10 04:34:12.627863708 +0000 UTC m=+245868.322003558: ☑️ agreed by JmPotato.
2026-02-10 05:31:43.475358566 +0000 UTC m=+249319.169498416: ☑️ agreed by lhy1024.

ti-chi-bot · 2026-02-10T10:29:38Z

@niubell: adding LGTM is restricted to approvers and reviewers in OWNERS files.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

ti-chi-bot · 2026-02-10T10:29:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JmPotato, lhy1024, niubell

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~~ [JmPotato,lhy1024]

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

okJiang · 2026-02-10T11:16:53Z

/pull-error-log-review

okJiang · 2026-02-10T11:17:11Z

/retest

okJiang · 2026-02-11T06:44:03Z

/retest

fix flaky test

147d25e

Signed-off-by: okjiang <819421878@qq.com>

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 3, 2026

server: prevent blocking on etcd close by handling it in the background

1590e0a

Signed-off-by: okjiang <819421878@qq.com>

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 4, 2026

Merge branch 'master' of https://github.com/tikv/pd into fix-10200

6b32f5c

JmPotato reviewed Feb 6, 2026

View reviewed changes

server: streamline startEtcd cleanup on errors

80ce0e5

Signed-off-by: okjiang <819421878@qq.com>

JmPotato approved these changes Feb 10, 2026

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Feb 10, 2026

lhy1024 approved these changes Feb 10, 2026

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Feb 10, 2026

niubell approved these changes Feb 10, 2026

View reviewed changes

ti-chi-bot bot merged commit ffaeac9 into tikv:master Feb 11, 2026
32 checks passed

coderabbitai bot mentioned this pull request Feb 11, 2026

server,tests: add server tso batch metric and test lock fix #10224

Open

test: fix flaky test TestForwardTestSuite in next-gen #10203

test: fix flaky test TestForwardTestSuite in next-gen #10203

Conversation

okJiang commented Feb 3, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

okJiang commented Feb 3, 2026

Uh oh!

codecov bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

okJiang commented Feb 4, 2026

Uh oh!

okJiang commented Feb 5, 2026

Uh oh!

JmPotato Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

okJiang Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

JmPotato Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot bot commented Feb 10, 2026

[LGTM Timeline notifier]

Uh oh!

ti-chi-bot bot commented Feb 10, 2026

Uh oh!

ti-chi-bot bot commented Feb 10, 2026

Uh oh!

okJiang commented Feb 10, 2026

Uh oh!

okJiang commented Feb 10, 2026

Uh oh!

okJiang commented Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

okJiang commented Feb 3, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Feb 3, 2026 •

edited

Loading

codecov bot commented Feb 4, 2026 •

edited

Loading