*: re-vendor kvproto at b41e863 (2026-06-22) and record the vendored revision#550
*: re-vendor kvproto at b41e863 (2026-06-22) and record the vendored revision#550eduralph wants to merge 1 commit 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
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (21)
📒 Files selected for processing (45)
📝 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 issues
🚥 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
from
a6fd942 to
ac3b830
Compare
…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. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Eduard R. <eduard@ralphovi.net>
eduralph
force-pushed
the
kvproto-refresh
branch
from
ac3b830 to
b01ca22
Compare
1 participant
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