Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a new PrepareBackup RPC for snapshot backup #1204

Merged
merged 10 commits into from
Jan 9, 2024

Conversation

YuJuncen
Copy link
Contributor

@YuJuncen YuJuncen commented Nov 3, 2023

Q & A

Some questions you may like to ask:

Q: Is it possible to split this into two RPCs?

Surely we can split this into two RPCs: one for ready and one for wait applies.

This will allow us to reuse some gRPC's infras to control connections, and easier to be understood the behavior.

But I don't want to do this because:

  • It seems waiting apply is meanless if we haven't disabled new commands proposing, which seems even harder to understand "why this RPC even exists?".
  • Controlling many asynchronous requests seems not really easier than a sole connection with many tagged request / responses.

@@ -15,6 +15,30 @@ option (rustproto.lite_runtime_all) = true;

option java_package = "org.tikv.kvproto";

enum PrepareSnapshotBackupRequestType {
UpdateLease = 0;
Copy link
Member

Choose a reason for hiding this comment

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

what's this used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating the TTL, I will add some comments then.

WaitApply = 1;
}

message PrepareSnapshotBackupRequest {
Copy link
Member

Choose a reason for hiding this comment

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

Please add some comments about the message and fields.

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

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

rest LGTM

proto/brpb.proto Outdated
@@ -15,6 +15,47 @@ option (rustproto.lite_runtime_all) = true;

option java_package = "org.tikv.kvproto";

enum PrepareSnapshotBackupRequestType {
// Update the lease of suspending new ingest / admin commands to be proposed.
Copy link
Member

Choose a reason for hiding this comment

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

Please describe what's the lease used for

Signed-off-by: hillium <[email protected]>
@YuJuncen YuJuncen merged commit 9326396 into pingcap:master Jan 9, 2024
4 checks passed
YuJuncen added a commit to YuJuncen/kvproto that referenced this pull request Jan 11, 2024
* added basic proto

Signed-off-by: hillium <[email protected]>

* finish

Signed-off-by: hillium <[email protected]>

* update new version of advanced prepare

Signed-off-by: hillium <[email protected]>

* added some comments and a new request type

Signed-off-by: hillium <[email protected]>

* added more comments

Signed-off-by: hillium <[email protected]>

* make go

Signed-off-by: hillium <[email protected]>

* address comments

Signed-off-by: hillium <[email protected]>

---------

Signed-off-by: hillium <[email protected]>
YuJuncen added a commit that referenced this pull request Jan 12, 2024
* added basic proto



* finish



* update new version of advanced prepare



* added some comments and a new request type



* added more comments



* make go



* address comments



---------

Signed-off-by: hillium <[email protected]>
YuJuncen added a commit to YuJuncen/kvproto that referenced this pull request Feb 20, 2024
* added basic proto

Signed-off-by: hillium <[email protected]>

* finish

Signed-off-by: hillium <[email protected]>

* update new version of advanced prepare

Signed-off-by: hillium <[email protected]>

* added some comments and a new request type

Signed-off-by: hillium <[email protected]>

* added more comments

Signed-off-by: hillium <[email protected]>

* make go

Signed-off-by: hillium <[email protected]>

* address comments

Signed-off-by: hillium <[email protected]>

---------

Signed-off-by: hillium <[email protected]>
YuJuncen added a commit that referenced this pull request Feb 20, 2024
)

* Added a new `PrepareBackup` RPC for snapshot backup (#1204)

* added basic proto

Signed-off-by: hillium <[email protected]>

* finish

Signed-off-by: hillium <[email protected]>

* update new version of advanced prepare

Signed-off-by: hillium <[email protected]>

* added some comments and a new request type

Signed-off-by: hillium <[email protected]>

* added more comments

Signed-off-by: hillium <[email protected]>

* make go

Signed-off-by: hillium <[email protected]>

* address comments

Signed-off-by: hillium <[email protected]>

---------

Signed-off-by: hillium <[email protected]>

* try trigger github action

Signed-off-by: hillium <[email protected]>

---------

Signed-off-by: hillium <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants