vars: validate some vars for tidb x (#68196)#68388
Merged
ti-chi-bot[bot] merged 3 commits intoMay 18, 2026
Merged
Conversation
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot
added
do-not-merge/hold
13 tasks
Member
Author
|
@ekexium This PR has conflicts, I have hold it. |
|
@ti-chi-bot: ## If you want to know how to resolve it, please read the guide in TiDB Dev Guide. 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 ti-community-infra/tichi repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR gates NextGen-incompatible features by restricting ChangesNextGen Compatibility Gating
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/realtikvtest/txntest/stale_read_test.go (1)
342-388:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winResolve merge-conflict markers in
TestStaleReadKVRequest(lines 342–388).The file contains unresolved conflict markers that make it invalid Go source, preventing compilation and test execution. Remove the conflict markers and choose the appropriate branch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/realtikvtest/txntest/stale_read_test.go` around lines 342 - 388, The file contains unresolved Git conflict markers inside TestStaleReadKVRequest causing a compile error; remove the conflict markers (<<<<<<<, =======, >>>>>>>) and reconcile the two variants: choose whether to keep the kerneltype.IsNextGen() conditional block (which uses NOW() - INTERVAL 1 SECOND and guards the three loops) or the original unconditional loops (which use NOW()), then ensure the loops that enable/disable failpoints (using failpoint.Enable/Disable), the START TRANSACTION / SET TRANSACTION statements, and the final follower-read assertion loops (tk.MustQuery / tk.MustExec / require.NoError) are consistent and compile; update only the conflicting section so function TestStaleReadKVRequest and calls like tk.MustExec, tk.MustQuery, failpoint.Enable/Disable remain valid and the file contains no conflict markers.
🧹 Nitpick comments (1)
pkg/sessionctx/variable/sysvar_test.go (1)
541-548: 💤 Low valueMinor: Reverse argument order for consistency.
Line 546 reverses the expected/actual order compared to line 544 and testify convention (
require.Equal(t, expected, actual, ...)).Style fix
} else { - require.Equal(t, val, "follower") + require.Equal(t, "follower", val) require.NoError(t, err) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/sessionctx/variable/sysvar_test.go` around lines 541 - 548, The test asserts in the non-nextgen branch use the wrong argument order for testify's require.Equal; change the call in the else branch so the expected value "follower" is passed first and the actual val second (i.e., swap arguments in the require.Equal call in the kerneltype.IsNextGen() else branch), keeping other checks like require.Error/require.NoError and ErrNotSupportedInNextGen comparisons unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DEPS.bzl`:
- Around line 7795-7811: Remove the Git merge conflict markers and keep the
incoming/cherry-picked client-go revision (the one with strip_prefix
"github.com/tikv/client-go/v2@v2.0.8-0.20260429090231-839228f8c022" and its
corresponding sha256
"239c5c831ab7ca4f27c0e2b3e73b2df5d828c38fb30ce2700c9d7f52b0ae4008") across the
affected files: update go.mod and go.sum to the incoming
v2.0.8-0.20260429090231-839228f8c022 entries, edit DEPS.bzl to remove the
<<<<<<</=======/>>>>>>> markers and keep the block with the 839228f8c022
strip_prefix/sha256/urls, and fix the two test files
(pkg/sessiontxn/staleread/provider_test.go and
tests/realtikvtest/txntest/stale_read_test.go) by removing conflict markers and
retaining the incoming changes from commit bdd978350e0; after edits run the
normal repo validation (go mod tidy && bazel/test commands) and then unhold per
ti-chi-bot.
In `@go.mod`:
- Around line 114-120: Resolve the unresolved merge markers by removing the
conflict blocks (the lines starting with <<<<<<<, =======, >>>>>>>) and keep
exactly one set of versions for the dependencies github.com/tikv/client-go/v2
and github.com/tikv/pd/client; choose the intended version pair (the one that
matches DEPS.bzl) and replace the conflicted block with those two clean
dependency lines so go.mod is valid and parses correctly.
In `@pkg/sessiontxn/staleread/provider_test.go`:
- Around line 88-89: Remove the unresolved Git merge conflict markers in
pkg/sessiontxn/staleread/provider_test.go (the sequences starting with <<<<<<<,
=======, and >>>>>>> at lines around 88–89 and 175) so the file is valid Go;
decide which of the conflicted blocks to keep (or merge their logic) and delete
the markers and the unwanted duplicate code, ensuring any referenced test
functions or helpers in provider_test.go remain correctly defined and the file
compiles.
---
Outside diff comments:
In `@tests/realtikvtest/txntest/stale_read_test.go`:
- Around line 342-388: The file contains unresolved Git conflict markers inside
TestStaleReadKVRequest causing a compile error; remove the conflict markers
(<<<<<<<, =======, >>>>>>>) and reconcile the two variants: choose whether to
keep the kerneltype.IsNextGen() conditional block (which uses NOW() - INTERVAL 1
SECOND and guards the three loops) or the original unconditional loops (which
use NOW()), then ensure the loops that enable/disable failpoints (using
failpoint.Enable/Disable), the START TRANSACTION / SET TRANSACTION statements,
and the final follower-read assertion loops (tk.MustQuery / tk.MustExec /
require.NoError) are consistent and compile; update only the conflicting section
so function TestStaleReadKVRequest and calls like tk.MustExec, tk.MustQuery,
failpoint.Enable/Disable remain valid and the file contains no conflict markers.
---
Nitpick comments:
In `@pkg/sessionctx/variable/sysvar_test.go`:
- Around line 541-548: The test asserts in the non-nextgen branch use the wrong
argument order for testify's require.Equal; change the call in the else branch
so the expected value "follower" is passed first and the actual val second
(i.e., swap arguments in the require.Equal call in the kerneltype.IsNextGen()
else branch), keeping other checks like require.Error/require.NoError and
ErrNotSupportedInNextGen comparisons unchanged.
🪄 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: 39895c3b-d905-4db6-a7e2-159fa7b17b97
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (18)
DEPS.bzlgo.modpkg/executor/distsql_test.gopkg/executor/executor_failpoint_test.gopkg/executor/test/distsqltest/BUILD.bazelpkg/executor/test/distsqltest/distsql_test.gopkg/session/test/variable/BUILD.bazelpkg/session/test/variable/variable_test.gopkg/sessionctx/variable/nextgen_test.gopkg/sessionctx/variable/sysvar.gopkg/sessionctx/variable/sysvar_test.gopkg/sessionctx/variable/varsutil_test.gopkg/sessiontxn/isolation/main_test.gopkg/sessiontxn/staleread/BUILD.bazelpkg/sessiontxn/staleread/provider_test.gotests/realtikvtest/sessiontest/session_fail_test.gotests/realtikvtest/txntest/replica_read_test.gotests/realtikvtest/txntest/stale_read_test.go
Signed-off-by: Ziqian Qin <eke@fastmail.com>
ti-chi-bot
Bot
added
size/XXL
ekexium
force-pushed
the
cherry-pick-68196-to-release-nextgen-20251011
branch
from
dbacd64 to
053bf09
Compare
Signed-off-by: Ziqian Qin <eke@fastmail.com>
ekexium
force-pushed
the
cherry-pick-68196-to-release-nextgen-20251011
branch
from
053bf09 to
e96123f
Compare
ti-chi-bot
Bot
added
size/XL
Member
|
/unhold |
ti-chi-bot
Bot
removed
the
do-not-merge/hold
zyguan
approved these changes
May 15, 2026
Member
|
/retest |
1 similar comment
Member
|
/retest |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-nextgen-20251011 #68388 +/- ##
=============================================================
Coverage ? 71.8467%
=============================================================
Files ? 1835
Lines ? 493715
Branches ? 0
=============================================================
Hits ? 354718
Misses ? 115603
Partials ? 23394
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
cfzjywxk
approved these changes
May 18, 2026
ti-chi-bot
Bot
added
cherry-pick-approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, yudongusa, zyguan 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
merged commit bd3a668
into
pingcap:release-nextgen-20251011
18 checks passed
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an automated cherry-pick of #68196
What problem does this PR solve?
Issue Number: close #60697
Problem Summary:
What changed and how does it work?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
Chores
New Features
Tests