Skip to content

Conversation

@hujiatao0
Copy link
Contributor

@hujiatao0 hujiatao0 commented Feb 3, 2026

What problem does this PR solve?

Issue Number: Close #xxx

What is changed and how does it work?

add PD CAS interface for key rotation

Check List

Tests

  • Unit test

Code changes

Related changes

Release note

None.

Summary by CodeRabbit

  • New Features
    • Added keyspace rotation workflow with three new API endpoints: start rotation, check rotation status, and complete rotation.
    • Includes structured error handling for rotation failures with clear classification of bad requests and conflicts.

Signed-off-by: hujiatao0 <hhjjtt110@gmail.com>
@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. labels Feb 3, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Feb 3, 2026

[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 rleungx 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 contribution This PR is from a community contributor. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Feb 3, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Feb 3, 2026

Hi @hujiatao0. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@ti-chi-bot ti-chi-bot bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 3, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

These changes introduce a keyspace rotation feature enabling controlled transitions between file IDs. The implementation spans manager APIs (StartRotation, CompleteRotation, GetRotationStatus), storage persistence, structured error handling, path utilities, and HTTP endpoints for the full workflow.

Changes

Cohort / File(s) Summary
Core Rotation Manager
pkg/keyspace/keyspace.go
Added StartRotation, GetRotationStatus, and CompleteRotation methods to Manager with request/response types. Validates NextFileID, performs CAS-like conflict detection, manages rotation state transitions, and logs outcomes.
Rotation Error Handling
pkg/keyspace/rotation_error.go
Introduced RotationErrorKind enum and RotationError type for structured error differentiation between bad requests and conflicts, with message formatting and error interface implementation.
Storage Persistence Layer
pkg/storage/endpoint/keyspace_rotation.go, pkg/storage/endpoint/keyspace.go
Extended KeyspaceStorage interface and implemented SaveKeyspaceRotation and LoadKeyspaceRotation methods on StorageEndpoint. Added KeyspaceRotationMeta struct for JSON-serialized rotation state.
Path Utilities
pkg/utils/keypath/absolute_key_path.go
Added KeyspaceRotationPath helper function to generate rotation metadata storage paths derived from keyspace ID.
HTTP API Endpoints
server/apiv2/handlers/keyspace.go
Introduced three rotation endpoints: POST /keyspaces/id/:id/rotation/start, GET /keyspaces/id/:id/rotation, POST /keyspaces/id/:id/rotation/complete. Added handlers with parameter binding, error mapping, and HTTP status translation.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant Handler as HTTP Handler
    participant Manager as Manager
    participant Storage as StorageEndpoint
    
    Client->>Handler: POST /keyspaces/id/:id/rotation/start<br/>(CurrentFileID, NextFileID)
    Handler->>Handler: Validate keyspace ID & parse params
    Handler->>Manager: StartRotation(StartRotationRequest)
    Manager->>Manager: Validate NextFileID vs CurrentFileID
    Manager->>Storage: LoadKeyspaceRotation(txn, keyspaceID)
    Storage-->>Manager: KeyspaceRotationMeta or nil
    Manager->>Manager: Create/update rotation state
    Manager->>Storage: SaveKeyspaceRotation(txn, meta)
    Storage-->>Manager: error or success
    Manager-->>Handler: RotationMeta or RotationError
    Handler->>Handler: Map error to HTTP status (400/409/500)
    Handler-->>Client: 200 RotationMeta or error response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Poem

🐰 A rotation tale we weave with care,
From file to file, with storage layer fair,
Three endpoints dance: start, check, and end,
Error handling whispers 'round each bend,
The keyspace spins, transitioning with grace! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'impl pd cas for key rotation' uses abbreviations (impl, pd, cas) and is vague about specifics, but it does reference the core change: implementing a CAS interface for key rotation. Consider expanding the title to be more explicit, e.g., 'Implement compare-and-swap (CAS) interface for keyspace rotation' for improved clarity and searchability.
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description follows the template structure with all required sections present, but the 'Issue Number' field contains a placeholder (Close #xxx) and the commit message is minimal.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Feb 3, 2026

[FORMAT CHECKER NOTIFICATION]

Notice: To remove the do-not-merge/needs-linked-issue label, please provide the linked issue number on one line in the PR body, for example: Issue Number: close #123 or Issue Number: ref #456, multiple issues should use full syntax for each issue and be separated by a comma, like: Issue Number: close #123, ref #456.

📖 For more info, you can check the "Linking issues" section in the CONTRIBUTING.md.

@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

❌ Patch coverage is 1.38889% with 213 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.37%. Comparing base (b668f43) to head (6769ae5).
⚠️ Report is 4 commits behind head on master.

❌ Your patch check has failed because the patch coverage (1.38%) is below the target coverage (74.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10208      +/-   ##
==========================================
- Coverage   78.59%   78.37%   -0.23%     
==========================================
  Files         520      522       +2     
  Lines       70014    70276     +262     
==========================================
+ Hits        55028    55079      +51     
- Misses      11008    11222     +214     
+ Partials     3978     3975       -3     
Flag Coverage Δ
unittests 78.37% <1.38%> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contribution This PR is from a community contributor. dco-signoff: yes Indicates the PR's author has signed the dco. do-not-merge/needs-linked-issue needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant