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.

Sign up for GitHub

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

Jump to bottom

tso: add SyncMaxTS RPC method #2988

Merged
merged 22 commits into from
Sep 27, 2020
Merged

tso: add SyncMaxTS RPC method #2988

merged 22 commits into from
Sep 27, 2020

Conversation

JmPotato
Copy link
Member

@JmPotato JmPotato commented Sep 18, 2020 •
edited
Loading

Signed-off-by: JmPotato [email protected]

What problem does this PR solve?

Part of #2984 and #2759. Prepare for the next step of implementing the global TSO generation algorithm.

What is changed and how it works?

Add SyncMaxTS RPC method.

Check List

Tests

  • Unit test
  • Integration test

Related changes

Release note

@JmPotato JmPotato added the component/tso Timestamp Oracle. label Sep 18, 2020
@JmPotato JmPotato self-assigned this Sep 18, 2020
Yisaer
Yisaer reviewed Sep 18, 2020
View reviewed changes
Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

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

Basically LGTM. I'm wondering maybe we also need to add the sender information like member ID in request. And each receiver should compare the sender member ID and the leader id in their vision.

server/grpc_service.go Outdated Show resolved Hide resolved
server/tso/allocator_manager.go Show resolved Hide resolved
@JmPotato
Copy link
Member Author

leader

Good idea. I will update this part later.

Yisaer
Yisaer previously approved these changes Sep 18, 2020
View reviewed changes
Copy link
Contributor

@Yisaer Yisaer left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 18, 2020
@JmPotato JmPotato mentioned this pull request Sep 19, 2020
Closed
16 tasks
@JmPotato
Copy link
Member Author

/run-all-tests

disksing
disksing reviewed Sep 21, 2020
View reviewed changes
server/grpc_service.go Outdated Show resolved Hide resolved
@Yisaer Yisaer dismissed their stale review September 21, 2020 06:34

plz address the comment

disksing
disksing reviewed Sep 21, 2020
View reviewed changes
server/grpc_service.go Outdated Show resolved Hide resolved
@disksing
Copy link
Contributor

disksing commented Sep 23, 2020 •
edited
Loading

update PR title?

@JmPotato JmPotato changed the title tso: add PrewriteMaxTS and WriteMaxTS RPC methods tso: add SyncMaxTS RPC method Sep 23, 2020
@JmPotato
Compact the RPC methods
d406988 
Signed-off-by: JmPotato <[email protected]>
disksing
disksing reviewed Sep 24, 2020
View reviewed changes
server/grpc_service.go Show resolved Hide resolved
server/grpc_service.go Show resolved Hide resolved
disksing
disksing reviewed Sep 25, 2020
View reviewed changes
server/grpc_service.go Outdated Show resolved Hide resolved
server/grpc_service.go Outdated Show resolved Hide resolved
server/grpc_service.go Show resolved Hide resolved
@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 27, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 27, 2020
@JmPotato
Copy link
Member Author

/run-all-tests

@JmPotato
Copy link
Member Author

/run-integration-lightning-test

@JmPotato JmPotato merged commit 427dd4c into tikv:master Sep 27, 2020
@JmPotato JmPotato deleted the tso_grpc branch September 27, 2020 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@disksing disksing disksing approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

@Yisaer Yisaer Awaiting requested review from Yisaer

Assignees

@JmPotato JmPotato

Labels
component/tso Timestamp Oracle. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JmPotato @disksing @Yisaer @ti-srebot