Skip to content

*: update kvproto to apiv3 - #2024

Open
disksing wants to merge 7 commits into
masterfrom
apiv3-update-proto
Open

*: update kvproto to apiv3#2024
disksing wants to merge 7 commits into
masterfrom
apiv3-update-proto

Conversation

@disksing

@disksing disksing commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

The kvproto APIV3 branch moves keyspace-related protobuf fields into oneofs, so the current client code no longer compiles against the latest definitions.

What changed?

  • Update kvproto to fd32127adca58438f6b05c869951182bbdd6041c in the root, integration-test, and example modules.
  • Update the PD client dependency to an APIV3-compatible revision.
  • Adapt API codec, region location, TiKV RPC, and tests to generated oneof getters and wrappers.

Tests

  • go test -tags=intest ./...

Summary by CodeRabbit

  • Compatibility
    • Updated keyspace-aware request/response handling to align with newer protobuf context/task meta shapes across MPP and compaction flows, including transaction and RPC context propagation.
    • Improved consistency by reading keyspace identifiers via the standard accessors throughout GC-related and test utilities.
  • Maintenance
    • Bumped the Go toolchain to 1.25.10 across examples and integration tooling.
    • Refreshed indirect dependencies, including gRPC/protobuf and related client libraries, to newer revisions.

Signed-off-by: disksing <i@disksing.com>
@ti-chi-bot ti-chi-bot Bot added the dco-signoff: yes Indicates the PR's author has signed the dco. label Jul 20, 2026
@ti-chi-bot

ti-chi-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nrc for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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 ti-chi-bot Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 20, 2026
@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@disksing, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ff35e131-1144-49a7-95b9-3f452a4d222c

📥 Commits

Reviewing files that changed from the base of the PR and between 96df3b6 and cfc77b3.

⛔ Files ignored due to path filters (2)
  • go.sum is excluded by !**/*.sum
  • integration_tests/go.sum is excluded by !**/*.sum
📒 Files selected for processing (10)
  • examples/gcworker/go.mod
  • examples/rawkv/go.mod
  • examples/txnkv/1pc_txn/go.mod
  • examples/txnkv/async_commit/go.mod
  • examples/txnkv/delete_range/go.mod
  • examples/txnkv/go.mod
  • examples/txnkv/pessimistic_txn/go.mod
  • examples/txnkv/unsafedestoryrange/go.mod
  • go.mod
  • integration_tests/go.mod
📝 Walkthrough

Walkthrough

Updates Go modules to Go 1.25.10 and newer dependency revisions, while migrating keyspace protobuf handling from direct fields to wrapper types and generated accessors across codecs, safe-point logic, integration tests, and test fixtures.

Changes

Keyspace compatibility and module updates

Layer / File(s) Summary
Protobuf keyspace request handling
internal/apicodec/codec.go, internal/apicodec/codec_v2_test.go, tikvrpc/tikvrpc_test.go
API context, compact, and MPP request construction and tests use wrapper-based keyspace fields and generated getters.
Keyspace accessor propagation
internal/apicodec/codec_v2.go, internal/locate/pd_codec.go, tikv/*, integration_tests/gc_test.go
Keyspace ID retrieval in codecs, PD handling, safe-point loading, integration tests, and test utilities uses GetId().
Go and dependency version refresh
go.mod, integration_tests/go.mod, examples/**/go.mod
Go directives move to 1.25.10 and kvproto, PD client, TiDB, client-go, gRPC, and protobuf dependencies are updated.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • tikv/pd#11025: Updates kvproto to APIV3 and migrates keyspace call sites to generated oneof APIs, matching this PR’s protobuf changes.
  • tikv/tikv#19856: Upgrades kvproto and adds compatibility changes for its updated protobuf API fields.

Possibly related PRs

Suggested reviewers: bufferflies, cfzjywxk, zyguan

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: upgrading kvproto for APIV3 compatibility.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch apiv3-update-proto

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

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
internal/apicodec/codec_v2_test.go (1)

887-887: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Swap arguments to match testify's expected vs. actual convention.

In testify/suite, the Equal method expects arguments in the order (expected, actual). Swapping them ensures that any failure messages will report the expected and actual values correctly instead of inversely.

♻️ Proposed refactor
-	suite.Equal(task.Meta.GetKeyspaceId(), testKeyspaceID)
+	suite.Equal(testKeyspaceID, task.Meta.GetKeyspaceId())
🤖 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 `@internal/apicodec/codec_v2_test.go` at line 887, Update the Equal assertion
in the relevant test to pass testKeyspaceID as the expected value and
task.Meta.GetKeyspaceId() as the actual value, preserving the existing
comparison.
🤖 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.

Nitpick comments:
In `@internal/apicodec/codec_v2_test.go`:
- Line 887: Update the Equal assertion in the relevant test to pass
testKeyspaceID as the expected value and task.Meta.GetKeyspaceId() as the actual
value, preserving the existing comparison.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 028004a3-d722-4842-84e1-09af2e9ec1ee

📥 Commits

Reviewing files that changed from the base of the PR and between 3d4b3ea and 1abe0db.

⛔ Files ignored due to path filters (2)
  • go.sum is excluded by !**/*.sum
  • integration_tests/go.sum is excluded by !**/*.sum
📒 Files selected for processing (17)
  • examples/gcworker/go.mod
  • examples/rawkv/go.mod
  • examples/txnkv/1pc_txn/go.mod
  • examples/txnkv/async_commit/go.mod
  • examples/txnkv/delete_range/go.mod
  • examples/txnkv/go.mod
  • examples/txnkv/pessimistic_txn/go.mod
  • examples/txnkv/unsafedestoryrange/go.mod
  • go.mod
  • integration_tests/go.mod
  • internal/apicodec/codec.go
  • internal/apicodec/codec_v2.go
  • internal/apicodec/codec_v2_test.go
  • internal/locate/pd_codec.go
  • tikv/compatible_txn_safe_point_loader.go
  • tikv/test_util.go
  • tikvrpc/tikvrpc_test.go

Signed-off-by: disksing <i@disksing.com>
@disksing

Copy link
Copy Markdown
Collaborator Author

Addressed the CodeRabbit nit in 901d142 by using the expected/actual argument order. This update also moves integration_tests to the APIV3-compatible TiDB revision and fixes the remaining KeyspaceMeta getter; the full integration test module now compiles.

disksing added 4 commits July 22, 2026 22:32
Signed-off-by: disksing <i@disksing.com>
Signed-off-by: disksing <i@disksing.com>
# Conflicts:
#	examples/gcworker/go.mod
#	examples/rawkv/go.mod
#	examples/txnkv/1pc_txn/go.mod
#	examples/txnkv/async_commit/go.mod
#	examples/txnkv/delete_range/go.mod
#	examples/txnkv/go.mod
#	examples/txnkv/pessimistic_txn/go.mod
#	examples/txnkv/unsafedestoryrange/go.mod
#	go.mod
#	go.sum
#	integration_tests/go.mod
#	integration_tests/go.sum
Signed-off-by: disksing <i@disksing.com>
@ti-chi-bot ti-chi-bot Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 24, 2026
Signed-off-by: disksing <i@disksing.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has signed the dco. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant