pkg/executor/test/unstabletest: stabilize flaky TestPBMemoryLeak

[flaky-claw]

What problem does this PR solve?

Issue Number: close #67422

Problem Summary:
Flaky test TestPBMemoryLeak in pkg/executor/test/unstabletest intermittently fails, so this PR stabilizes that path.

What changed and how does it work?

Root Cause

TEST_ISSUE; TestPBMemoryLeak treated first-scan warm-up allocations and retained heap noise as a leak signal, making the original HeapInuse assertion sensitive to one-time setup effects rather than repeated-scan growth.

Fix

Reworked the test to treat the first scan as warm-up, close result sets, and assert that HeapInuse does not keep growing on a second scan; also enabled intest assertions per test with cleanup and added the Bazel dep for the new import.

Verification

Spec:

• target: pkg/executor/test/unstabletest :: TestPBMemoryLeak
• strategy: tidb.go_flaky.default
• plan mode: BASELINE_ONLY
• requirements: required case must execute; no skip; repeat count = 1
• baseline gates: required_flaky_gate, build_safety_gate, intent_guard_gate

Observed result:

• status: passed
• required case executed: yes
• submission decision: ALLOWED
• note: Required flaky case executed during validation.
  Required flaky case was not skipped.
  Required flaky gate passed.
  Affected scope regression gate passed.
  Build safety gate passed.
  Intent guard gate passed.

Gate checklist:

• Required flaky gate: PASS
• Affected scope regression gate: PASS
• Build safety gate: PASS
• Intent guard gate: PASS
• Repo-wide advisory gate: SKIPPED
• Feedback specific gate: SKIPPED

Commands:

• go test -json ./pkg/executor/test/unstabletest -run '^TestPBMemoryLeak$' -count=1
• go test -json ./pkg/executor/test/unstabletest -count=1
• make build

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Fixes #67422

Summary by CodeRabbit

• Tests
  • Improved memory-leak detection with stronger measurement and additional runtime assertions.
• Refactor
  • Reorganized test flow and introduced safer resource cleanup for more reliable test execution.
• Chores
  • Expanded test build configuration to include an additional test-time dependency.

[pantheon-ai]

@flaky-claw I've received your pull request and will start the review. I'll conduct a thorough review covering code quality, potential issues, and implementation details.

⏳ This process typically takes 10-30 minutes depending on the complexity of the changes.

_{ℹ️ Learn more details on Pantheon AI.}

[tiprow]

Hi @flaky-claw. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

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.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 813a1935-2c49-41f3-af18-71c813a4d7f8

📥 Commits

Reviewing files that changed from the base of the PR and between cee391e and c09443f.

📒 Files selected for processing (2)

• pkg/executor/test/unstabletest/BUILD.bazel
• pkg/executor/test/unstabletest/memory_test.go

✅ Files skipped from review due to trivial changes (1)

• pkg/executor/test/unstabletest/BUILD.bazel

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

• pkg/executor/test/unstabletest/memory_test.go

---

📝 Walkthrough

Walkthrough

Added //pkg/util/intest to the Bazel test target deps; enabled intest assertions in two tests; added readAllRowsAndClose helper; and replaced three-point memory-leak checks with a warmup scan + GC + second-scan heap-growth comparison.

Changes

Unstable test refactor

Layer / File(s)	Summary
Build Dependency pkg/executor/test/unstabletest/BUILD.bazel	Added //pkg/util/intest to unstabletest_test deps.
Test imports & enablement pkg/executor/test/unstabletest/memory_test.go	Imported pkg/util/intest and call enableIntestAssertForTest(t) at start of TestGlobalMemoryControl and TestPBMemoryLeak.
Helper: read rows and close pkg/executor/test/unstabletest/memory_test.go	Added readAllRowsAndClose(t, tk, sql) to execute a query, iterate all rows, count them, and close the result set.
Memory-leak measurement logic pkg/executor/test/unstabletest/memory_test.go	Replaced three-point memDiff checks with a warmup scan that asserts warmup allocation increased, then GC and a second scan; compute heap-growth (clamp negative to zero) and assert growth < delta.
Enable helper restore function pkg/executor/test/unstabletest/memory_test.go	Added enableIntestAssertForTest(t) implementation to toggle intest flags and restore previous values via t.Cleanup.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• hawkingrei
• xhebox

Poem

🐇 I hopped into tests at dawn,
I nudged assertions gently on.
Warmup, GC, a second peek—
Heap growth tiny, outcomes meek.
Tiny carrots, memory strong. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: stabilizing a flaky test (TestPBMemoryLeak) in the specified package (pkg/executor/test/unstabletest).
Description check	✅ Passed	The description includes all required sections: linked issue (#67422), problem summary explaining the root cause and fix, detailed explanation of what changed, completed checklist (unit test selected), no side effects marked, and release notes marked as None.
Linked Issues check	✅ Passed	The PR directly addresses issue #67422 by stabilizing the flaky TestPBMemoryLeak test through reworked heap-growth assertions, intest enablement, and added Bazel dependencies, meeting the stated objectives of reducing intermittent failures.
Out of Scope Changes check	✅ Passed	All changes are scoped to stabilizing TestPBMemoryLeak: BUILD.bazel dependency addition, test refactoring, and memory-leak verification logic rework are directly aligned with the linked issue's objectives.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/executor/test/unstabletest/memory_test.go`:
- Around line 142-144: The test currently uses memDiff (absolute) which fails
when HeapInuse drops; instead compute positive growth only by taking growth :=
int64(inUseAfterSecondScan.HeapInuse) - int64(inUseAfterWarmup.HeapInuse), clamp
growth to zero if negative (if growth < 0 { growth = 0 }), and then assert
require.Less(t, growth, delta) (or equivalent) so only continued upward heap
growth beyond delta fails; update the assertion replacing memDiff(...) with this
positive-growth check referring to readMem, inUseAfterSecondScan,
inUseAfterWarmup, and HeapInuse.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6c2808e7-db37-41f7-92d1-02e685a10f15

📥 Commits

Reviewing files that changed from the base of the PR and between ce92298 and 170d61e.

📒 Files selected for processing (2)

• pkg/executor/test/unstabletest/BUILD.bazel
• pkg/executor/test/unstabletest/memory_test.go

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.0163%. Comparing base (2d78fa9) to head (c09443f).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #67880        +/-   ##
================================================
- Coverage   77.7307%   77.0163%   -0.7144%     
================================================
  Files          1990       1973        -17     
  Lines        551899     552971      +1072     
================================================
- Hits         428995     425878      -3117     
- Misses       121984     127064      +5080     
+ Partials        920         29       -891     

Flag	Coverage Δ
integration	41.3583% <ø> (+1.5565%)	⬆️

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

Components	Coverage Δ
dumpling	60.4888% <ø> (ø)
parser	∅ <ø> (∅)
br	50.0597% <ø> (-13.0338%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[yinsustart]

/check-issue-triage-complete

[flaky-claw]

FlakyFixer PR update summary

Fix

• The test must only fail on continued upward heap growth after the warmup scan, so the final assertion needed to ignore negative growth instead of measuring absolute movement.

Verification

Spec:

• target: pkg/executor/test/unstabletest :: TestPBMemoryLeak
• strategy: tidb.go_flaky.default
• plan mode: BASELINE_PLUS_FEEDBACK_DELTA
• requirements: required case must execute; no skip; repeat count = 1
• baseline gates: required_flaky_gate, build_safety_gate, intent_guard_gate
• feedback delta: 1 finding(s) via feedback_specific_gate (non-blocking fallback)
• feedback surface source: baseline_only

Observed result:

• status: passed
• required case executed: yes
• submission decision: ALLOWED
• note: Required flaky case executed during validation.
  Required flaky case was not skipped.
  Required flaky gate passed.
  Affected scope regression gate passed.
  Build safety gate passed.
  Intent guard gate passed.

Gate checklist:

• Required flaky gate: PASS
• Affected scope regression gate: PASS
• Build safety gate: PASS
• Intent guard gate: PASS
• Repo-wide advisory gate: SKIPPED
• Feedback specific gate: SKIPPED

Commands:

• go test -json ./pkg/executor/test/unstabletest -run '^TestPBMemoryLeak$' -count=1
• go test -json ./pkg/executor/test/unstabletest -count=1
• make build

[yinsustart]

/retest

[tiprow]

@yinsustart: PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test.

Details

In response to this:

/retest

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]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Defined2014, hawkingrei

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 [Defined2014,hawkingrei]

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-05-07 02:44:22.123494433 +0000 UTC m=+322134.996844405: ☑️ agreed by hawkingrei.
• 2026-05-07 05:15:35.707623902 +0000 UTC m=+331208.580973894: ☑️ agreed by Defined2014.

[tiprow]

@flaky-claw: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
fast_test_tiprow	cee391e	link	true	/test fast_test_tiprow

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.