br/pkg/stream/crr/internal/checkpoint: stabilize flaky TestCheckpointCalculatorRandomizedCRRSimulation

[flaky-claw]

What problem does this PR solve?

Issue Number: close #67699

Problem Summary:
Flaky test TestCheckpointCalculatorRandomizedCRRSimulation in br/pkg/stream/crr/internal/checkpoint intermittently fails, so this PR stabilizes that path.

What changed and how does it work?

Root Cause

TestCheckpointCalculatorRandomizedCRRSimulation was flaky because unconditional per-round info logging made the randomized test exceed CI slow-test thresholds under scheduler pressure; this was a test-side timing issue, not a product bug.

Fix

The patch removes only the non-essential round trace from normal runs by making it opt-in through TIDB_RANDOMIZED_CRR_SIM_ROUND_LOG, which cuts timing variance without weakening any assertions.

Verification

Spec:

• target: br/pkg/stream/crr/internal/checkpoint :: TestCheckpointCalculatorRandomizedCRRSimulation
• 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
• scope debt present: yes

Gate checklist:

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

Commands:

• go test -json ./br/pkg/stream/crr/internal/checkpoint -run '^TestCheckpointCalculatorRandomizedCRRSimulation$' -count=1
• go test -json ./br/pkg/stream/crr/internal/checkpoint -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 #67699

Summary by CodeRabbit

• Tests
  • Added environment variable control for per-round test logging, allowing verbose output to be toggled to reduce log noise during test runs.
  • Reduced randomized simulation iterations (from 1000 to 300) to shorten test execution time and make runs faster and more practical for routine testing.

[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]

📝 Walkthrough

Walkthrough

Replaced unconditional per-round logging with an environment-variable guard (TIDB_RANDOMIZED_CRR_SIM_ROUND_LOG) and reduced randomizedCRRSimulationConfig.Iterations from 1000 to 300 in the randomized CRR simulation test file (imports updated to include os).

Changes

Cohort / File(s)	Summary
Randomized CRR simulation test br/pkg/stream/crr/internal/checkpoint/randomized_integration_test.go	Added os import; wrapped per-round s.log(...) with a check of TIDB_RANDOMIZED_CRR_SIM_ROUND_LOG; reduced randomizedCRRSimulationConfig.Iterations from 1000 to 300.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

size/S, ok-to-test, lgtm

Suggested reviewers

• YuJuncen
• Leavrth

Poem

🐰 I bounded through tests with a curious hop,

Logs muffled by an env var—no nonstop—
Rounds trimmed to three hundred, breezy and light,
Quiet as clover, yet ready to write. 🥕✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 in the specified package by addressing timing issues.
Description check	✅ Passed	The PR description follows the template with issue reference, problem summary, detailed explanation of changes, and completed checklist.
Linked Issues check	✅ Passed	The PR directly addresses issue #67699 by implementing stabilization measures: making per-round logging opt-in via environment variable and reducing iteration count.
Out of Scope Changes check	✅ Passed	All changes are within scope: logging modification in randomizedCRRSimulation.runRound, os import addition, and iteration count reduction directly address test flakiness.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

Command failed

---

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.5801%. Comparing base (65d9fb6) to head (7b2c600).
⚠️ Report is 83 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #67826        +/-   ##
================================================
+ Coverage   77.5964%   78.5801%   +0.9837%     
================================================
  Files          1982       1993        +11     
  Lines        548885     562697     +13812     
================================================
+ Hits         425915     442168     +16253     
+ Misses       122165     118887      -3278     
- Partials        805       1642       +837     

Flag	Coverage Δ
integration	45.4037% <ø> (+11.0637%)	⬆️
unit	76.6583% <ø> (+0.3178%)	⬆️

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

Components	Coverage Δ
dumpling	60.4888% <ø> (-0.9277%)	⬇️
parser	∅ <ø> (∅)
br	66.0316% <ø> (+5.5073%)	⬆️

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

[YuJuncen]

Better to also reduce test rounds.

[pingcap-cla-assistant]

All committers have signed the CLA.

[yinsustart]

@YuJuncen, I reduced Iterations to 300 because this test still needs enough rounds to exercise at least one full catch-up cycle. With the current config, CatchUpEvery is 299, so lowering iterations to 100 can skip catch-up entirely and leave SyncedTS at 0, causing the randomized simulation to fail for some seeds. 300 is the minimum value assuming no other changes to the logic.

[coderabbitai]

🧹 Nitpick comments (1)

br/pkg/stream/crr/internal/checkpoint/randomized_integration_test.go (1)

37-38: Avoid weakening the randomized coverage unless the logging fix is insufficient.

With CatchUpEvery: 299, Iterations: 300 exercises the catch-up block only once. Since the reported root cause is per-round logging, consider keeping the original iteration count so this PR stays focused on removing the timing overhead without reducing simulation depth.

Proposed adjustment

 	cfg := randomizedCRRSimulationConfig{
-		Iterations:                        300,
+		Iterations:                        1000,

As per coding guidelines, keep test changes minimal and deterministic; avoid broad test churn unless required.

🤖 Prompt for AI Agents

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

In `@br/pkg/stream/crr/internal/checkpoint/randomized_integration_test.go` around
lines 37 - 38, The test weakens randomized coverage by setting CatchUpEvery: 299
while keeping Iterations: 300 so the catch-up path runs only once; update the
randomizedCRRSimulationConfig so Iterations is restored to its original/higher
value (or at least substantially > CatchUpEvery, e.g., >= CatchUpEvery*3) to
ensure multiple catch-up rounds are exercised, keeping the rest of the
logging/timing change minimal and deterministic.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@br/pkg/stream/crr/internal/checkpoint/randomized_integration_test.go`:
- Around line 37-38: The test weakens randomized coverage by setting
CatchUpEvery: 299 while keeping Iterations: 300 so the catch-up path runs only
once; update the randomizedCRRSimulationConfig so Iterations is restored to its
original/higher value (or at least substantially > CatchUpEvery, e.g., >=
CatchUpEvery*3) to ensure multiple catch-up rounds are exercised, keeping the
rest of the logging/timing change minimal and deterministic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: cb609bdd-0095-4666-8a72-da0fed8b2ead

📥 Commits

Reviewing files that changed from the base of the PR and between 743fe5b and 7b2c600.

📒 Files selected for processing (1)

• br/pkg/stream/crr/internal/checkpoint/randomized_integration_test.go

[yinsustart]

/check-issue-triage-complete

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Leavrth, YuJuncen

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:

• br/OWNERS [Leavrth,YuJuncen]

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-04-17 09:08:48.920856352 +0000 UTC m=+1724934.126216409: ☑️ agreed by YuJuncen.
• 2026-04-28 05:00:49.831256768 +0000 UTC m=+2660455.036616815: ☑️ agreed by Leavrth.

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

[hawkingrei]

/retest

[hawkingrei]

/retest

[hawkingrei]

/retest

[hawkingrei]

/retest

[hawkingrei]

/retest

[hawkingrei]

/retest

[hawkingrei]

/retest

[hawkingrei]

/retest

[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	7b2c600	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.