Repository navigation
tests: improve timing robustness in TestStaleReadAllCombinations - #63196
Conversation
Fix flakiness in TestStaleReadAllCombinations/tidb_read_staleness under high CI load by: - Increasing sleep intervals from 1s to 3s for better timing separation - Adjusting timestamp offsets proportionally to maintain test logic - Increasing staleness buffer from +1s to +2s for more timing margin 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: ekexium <eke@fastmail.com>
ti-chi-bot
Bot
added
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
release-note-none
hawkingrei
added
the
skip-issue-check
ti-chi-bot
Bot
removed
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
labels
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #63196 +/- ##
================================================
+ Coverage 72.8055% 73.5182% +0.7127%
================================================
Files 1828 1844 +16
Lines 494865 518436 +23571
================================================
+ Hits 360289 381145 +20856
- Misses 112689 115021 +2332
- Partials 21887 22270 +383
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/retest |
|
/retest-required |
| time.Sleep(1000 * time.Millisecond) | ||
| firstTime := time.Now().Add(-500 * time.Millisecond) | ||
| time.Sleep(3000 * time.Millisecond) // Increased from 1s to 3s for better timing separation | ||
| firstTime := time.Now().Add(-1500 * time.Millisecond) // Adjusted offset |
There was a problem hiding this comment.
What about calculating the ts from PD tso?
| firstTime := time.Now().Add(-1500 * time.Millisecond) // Adjusted offset | |
| ts, err := tk.Session().GetStore().GetOracle().GetTimestamp(context.Background(), &oracle.Option{}) | |
| require.NoError(t, err) | |
| firstTime := oracle.GetTimeFromTS(ts).Add(-1500 * time.Millisecond) // Adjusted offset |
| time.Sleep(1000 * time.Millisecond) | ||
| secondTime := time.Now().Add(-500 * time.Millisecond) | ||
| time.Sleep(3000 * time.Millisecond) // Increased from 1s to 3s | ||
| secondTime := time.Now().Add(-1500 * time.Millisecond) // Adjusted offset |
There was a problem hiding this comment.
Ditto.
| secondTime := time.Now().Add(-1500 * time.Millisecond) // Adjusted offset | |
| ts, err = tk.Session().GetStore().GetOracle().GetTimestamp(context.Background(), &oracle.Option{}) | |
| require.NoError(t, err) | |
| secondTime := oracle.GetTimeFromTS(ts).Add(-1500 * time.Millisecond) // Adjusted offset |
| staleness := row2CreatedElapsed + 1 // The time `now - staleness(second)` is between row1 and row2. | ||
| // Ensure sufficient buffer: with 3s sleep intervals, elapsed should be ~3s | ||
| // Adding 2s buffer ensures stale read is safely before row2 insertion | ||
| staleness := row2CreatedElapsed + 2 |
There was a problem hiding this comment.
And get the row2CreatedElapsed from PD tso.
| tk.MustExec("insert into t values (1, 10)") | ||
| time.Sleep(1000 * time.Millisecond) | ||
| firstTime := time.Now().Add(-500 * time.Millisecond) | ||
| time.Sleep(3000 * time.Millisecond) // Increased from 1s to 3s for better timing separation |
There was a problem hiding this comment.
can we use failpoint to add some hook, to make sure async actions are done?
|
Looks like stale read tests are stabilized now in CI. Close this for now. Reconsider these change when needed |
|
/check-issue-triage-complete |
ti-chi-bot
Bot
added
size/M
|
I guess the key is not about timing, but slow executions of so many permutations affecting |
0478b57 to
5d2d713
Compare
Signed-off-by: ekexium <eke@fastmail.com>
5d2d713 to
f67de14
Compare
|
/retest-required |
Signed-off-by: ekexium <eke@fastmail.com>
ti-chi-bot
Bot
added
approved
needs-1-more-lgtm
| if err != nil { | ||
| t.Fatalf("failed to get TSO: %v", err) | ||
| } | ||
| time.Sleep(3000 * time.Millisecond) // Increased from 1s to 3s for better timing separation |
There was a problem hiding this comment.
Is the purpose to ensure the firstTime to be after the insertion? How about using tidb_last_txn_info to get the commit ts of the insertion, and wait until the current ts - the gap exceed the commit ts? I guess in this way we won't need to use up the 3s when the test environment is stable enough.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MyonKeminta, you06 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
/retest-required |
3 similar comments
|
/retest-required |
|
/retest-required |
|
/retest-required |
|
@ekexium: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
6 participants
What problem does this PR solve?
Issue Number: close #63271
Problem Summary:
The
TestStaleReadAllCombinations/tidb_read_stalenesstest case is flaky under high CI load due to tight timing constraints.What changed and how does it work?
Changes made:
tidb_read_stalenesscases firstCheck List
Tests
Side effects
Documentation
Release note