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

tso: Fix Local tso alloactor become alloactor leader without pd leader knowing its dc-location #3134

Merged
merged 22 commits into from
Nov 5, 2020

Conversation

Yisaer
Copy link
Contributor

@Yisaer Yisaer commented Nov 2, 2020

Signed-off-by: Song Gao disxiaofei@163.com

What problem does this PR solve?

fix #3124
ref pingcap/kvproto#688

What is changed and how it works?

During allocatorLeaderLoop, the Allocator should query the dcLocations from leader to be sured that the pd leader is aware of this dc-location.

Check List

Tests

  • Unit test

Release note

Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
@Yisaer Yisaer requested a review from JmPotato November 2, 2020 08:51
@Yisaer Yisaer added component/tso Timestamp Oracle. type/bugfix This PR fixes a bug. labels Nov 2, 2020
Signed-off-by: Song Gao <disxiaofei@163.com>
@Yisaer Yisaer requested a review from disksing November 2, 2020 09:08
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
server/tso/global_allocator.go Outdated Show resolved Hide resolved
server/tso/allocator_manager.go Show resolved Hide resolved
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Copy link
Member

@JmPotato JmPotato 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, could you please add a test for this feature then?

Signed-off-by: Song Gao <disxiaofei@163.com>
go.mod Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 3, 2020
Signed-off-by: Song Gao <disxiaofei@163.com>
@Yisaer Yisaer added the require-LGT1 Indicates that the PR requires an LGTM. label Nov 4, 2020
@Yisaer
Copy link
Contributor Author

Yisaer commented Nov 4, 2020

/merge

@ti-chi-bot
Copy link
Member

@Yisaer: you cannot merge your own PR.

In response to this:

/merge

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 tidb-community-bots/prow-config repository.

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 4, 2020
@HunDunDM
Copy link
Member

HunDunDM commented Nov 4, 2020

/merge

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 4, 2020
@ti-chi-bot
Copy link
Member

Can merge label has been added.

Git tree hash: 0743edebc1ce6125831caaa1c1b79e59d9f370bd

@HunDunDM
Copy link
Member

HunDunDM commented Nov 4, 2020

/rebase

@HunDunDM
Copy link
Member

HunDunDM commented Nov 4, 2020

/run-all-tests

Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
@ti-chi-bot
Copy link
Member

New changes are detected. Can merge label has been removed.

@ti-chi-bot ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Nov 4, 2020
@HunDunDM
Copy link
Member

HunDunDM commented Nov 4, 2020

/run-all-tests

1 similar comment
@JmPotato
Copy link
Member

JmPotato commented Nov 4, 2020

/run-all-tests

@Yisaer
Copy link
Contributor Author

Yisaer commented Nov 4, 2020

/run-all-tests

@Yisaer
Copy link
Contributor Author

Yisaer commented Nov 4, 2020

/merge

@ti-chi-bot
Copy link
Member

@Yisaer: you cannot merge your own PR.

In response to this:

/merge

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 tidb-community-bots/prow-config repository.

@Yisaer Yisaer merged commit 8f3ce48 into tikv:master Nov 5, 2020
longfangsong pushed a commit to longfangsong/pd that referenced this pull request Nov 5, 2020
…r knowing its dc-location (tikv#3134)

Signed-off-by: longfangsong <longfangsong@icloud.com>
ti-chi-bot added a commit that referenced this pull request Nov 27, 2020
* tso: Fix Local tso alloactor become alloactor leader without pd leader knowing its dc-location (#3134)

Signed-off-by: longfangsong <longfangsong@icloud.com>

* persist to etcd

Signed-off-by: longfangsong <longfangsong@icloud.com>

* load on boot and change leader

Signed-off-by: longfangsong <longfangsong@icloud.com>

* Apply several suggestion from code review

Signed-off-by: longfangsong <longfangsong@icloud.com>

* extract getTTLUint and getTTLFloat

Signed-off-by: longfangsong <longfangsong@icloud.com>

* extract EtcdKVPutWithTTL

Signed-off-by: longfangsong <longfangsong@icloud.com>

* format with goimports

Signed-off-by: longfangsong <longfangsong@icloud.com>

* support leader-schedule-limit

Signed-off-by: longfangsong <longfangsong@icloud.com>

* add tests

Signed-off-by: longfangsong <longfangsong@icloud.com>

* support hot-region-schedule-limit & replica-schedule-limit & merge-schedule-limit

Signed-off-by: longfangsong <longfangsong@icloud.com>

* add integration test and fix a bug

Signed-off-by: longfangsong <longfangsong@icloud.com>

* cleanup tests

Signed-off-by: longfangsong <longfangsong@icloud.com>

* Apply some suggestions from code review

Signed-off-by: longfangsong <longfangsong@icloud.com>

* extract getTTLFloatOr

Signed-off-by: longfangsong <longfangsong@icloud.com>

* apply suggestions from code review

Signed-off-by: longfangsong <longfangsong@icloud.com>

* apply suggestions from code review

Signed-off-by: longfangsong <longfangsong@icloud.com>

* apply suggestions from code review

Signed-off-by: longfangsong <longfangsong@icloud.com>

* make TestConfigTTLAfterTransferLeader run Serial to prevent it affect TestScatterRegion

Signed-off-by: longfangsong <longfangsong@icloud.com>

* apply suggestions from code reviewing

Signed-off-by: longfangsong <longfangsong@icloud.com>

* apply suggestions in code review

Signed-off-by: longfangsong <longfangsong@icloud.com>

* make the testing time as little as possible

Signed-off-by: longfangsong <longfangsong@icloud.com>

Co-authored-by: Song Gao <disxiaofei@163.com>
Co-authored-by: Ti Prow Robot <71242396+ti-community-prow-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tso Timestamp Oracle. require-LGT1 Indicates that the PR requires an LGTM. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Local TSO Allocator might be unknown for PD Leader after working
4 participants