*: re-vendor kvproto at b41e863 (2026-06-22) and record the vendored revision#550
Open
eduralph wants to merge 2 commits into
Open
*: re-vendor kvproto at b41e863 (2026-06-22) and record the vendored revision#550eduralph wants to merge 2 commits into
eduralph wants to merge 2 commits into
Conversation
ti-chi-bot
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
contribution
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (21)
📒 Files selected for processing (45)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughThe update vendors a newer kvproto revision, expands protobuf service and message contracts, standardizes code-generation options, and changes Rust retry and lock handling for undetermined results and shared-lock wrappers. ChangesProtocol and transaction safety changes
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
eduralph
force-pushed
the
kvproto-refresh
branch
4 times, most recently
from
July 18, 2026 12:00
ccb481a to
0bc204c
Compare
Contributor
Author
|
/retest |
|
@eduralph: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn 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. |
…revision The vendored protos were an unrecorded late-2023/early-2024 vintage (errorpb and pdpb untouched since tikv#324, one hand-patched addition from tikv#519). Re-vendor everything at pingcap/kvproto b41e86365ce0 — the revision client-go currently pins — and add proto/VERSION so the vintage is never again guesswork. Regenerated with the existing tonic-build pipeline (21 files). One mechanical source fix: pdpb.RequestHeader gained caller_component/caller_id. Two new protocol semantics arrive with the refresh and are handled rather than silently mis-parsed: 1. errorpb.UndeterminedResult (an unknown raft apply outcome). The transport never retries it in client-go (region_request.go: 'should not retry ... processed by the caller'); each action decides. Here: the plan layer retries by default (replaying an idempotent request resolves the uncertainty, and the error escapes UNCHANGED on backoff exhaustion), while plans for which a replay is unsafe are terminal on first sight — raw CAS (a replay would compare against its own effect and report a false failure), the primary commit (client-go returns ErrResultUndetermined there, commit.go), and async-commit/1PC prewrites (the commit point; stricter than client-go, in the safe direction). Commit paths classify the surfaced error as UndeterminedError: at a commit point, 'failed' must never be claimed when the transaction may already be durable. 2. Shared locks (SharedLock / SharedPessimisticLock, whose real holders live in shared_lock_infos — 'DO NOT read from the wrapper LockInfo'). This client does not implement shared-lock resolution yet, and partial handling is worse than none (resolving the wrapper checks transaction 0; filtering on wrapper fields silently drops members). Resolution therefore REFUSES them with an explicit error, before any filter. The API-v2 keyspace codecs are wrapper-aware (identifying wrappers by TYPE — the empty logical key is valid — and recursing into members), so scan results can never panic the truncation path. Tests: 67 lib tests (4 new: undetermined terminal/preserved/recognized, shared-lock refusal); txn/raw/failpoint integration suites green. Signed-off-by: Eduard R. <eduard@ralphovi.net>
eduralph
force-pushed
the
kvproto-refresh
branch
from
July 18, 2026 12:37
0bc204c to
713dc42
Compare
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
2 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.
Why: the vendored protos are an unrecorded late-2023/early-2024 vintage (
errorpb.proto/pdpb.protountouched since #324, one hand-patched addition from #519). Nothing records which kvproto revision they came from, and the staleness now blocks real features.What: all vendored protos + the include tree re-vendored at pingcap/kvproto
b41e863— the revision client-go pins today — plus a newproto/VERSIONrecording it. Regenerated with the existing tonic-build pipeline (21 files); one mechanical source fix (pdpb.RequestHeadergainedcaller_component/caller_id). Newly available surface includesKvFlush/KvBufferBatchGet(pipelined DML),HealthFeedback, andpdpb.BatchScanRegions.Two new protocol semantics arrive with the refresh and are handled rather than silently mis-parsed:
errorpb.UndeterminedResult(an unknown raft apply outcome). Following client-go's shape — its transport never retries this ("should not retry … processed by the caller",region_request.go) and each action decides — the plan layer here retries by default (replaying an idempotent request resolves the uncertainty; the error escapes unchanged on backoff exhaustion), while plans for which a replay is unsafe are terminal on first sight: raw CAS (a replay would compare against its own effect and reportsucceed = falsefor a write that happened), the primary commit (client-go returnsErrResultUndeterminedthere,commit.go), and async-commit/1PC prewrites (the commit point — stricter than client-go, in the safe direction). Commit paths classify the surfaced error asUndeterminedError: at a commit point, "failed" must never be claimed when the transaction may already be durable.Shared locks (
SharedLock/SharedPessimisticLock; real holders live inshared_lock_infos— "DO NOT read from the wrapper"). This client doesn't implement shared-lock resolution yet, and partial handling is worse than none (resolving the wrapper checks transaction 0; filtering on wrapper fields silently drops members). Resolution refuses them with an explicit error, and the API-v2 keyspace codecs are wrapper-aware (keyed on type — the empty logical key is valid — recursing into members), so scan results can't panic the truncation path. Full shared-lock support is deliberately follow-up work.Verification:
make checkgreen; 67 lib tests (4 new covering the undetermined and shared-lock semantics); txn/raw/failpoint integration suites green against a local api-v2 cluster; and behavior compared against client-go by driving both clients through identical transactional and raw scenarios — no observable change.Summary by CodeRabbit