Skip to content

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

Merged
Yisaer merged 22 commits into
tikv:masterfrom
Yisaer:fix_tso_unknown
Nov 5, 2020
Merged

tso: Fix Local tso alloactor become alloactor leader without pd leader knowing its dc-location#3134
Yisaer merged 22 commits into
tikv:masterfrom
Yisaer:fix_tso_unknown

Conversation

@Yisaer
Copy link
Copy Markdown
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>
Comment thread server/tso/global_allocator.go Outdated
Comment thread server/tso/allocator_manager.go
Signed-off-by: Song Gao <disxiaofei@163.com>
Signed-off-by: Song Gao <disxiaofei@163.com>
Copy link
Copy Markdown
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>
Comment thread go.mod Outdated
@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
Copy Markdown
Contributor Author

Yisaer commented Nov 4, 2020

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

@Yisaer: you cannot merge your own PR.

Details

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
Copy Markdown
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
Copy Markdown
Member

Can merge label has been added.

DetailsGit tree hash: 0743edebc1ce6125831caaa1c1b79e59d9f370bd

@HunDunDM
Copy link
Copy Markdown
Member

HunDunDM commented Nov 4, 2020

/rebase

@HunDunDM
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

HunDunDM commented Nov 4, 2020

/run-all-tests

1 similar comment
@JmPotato
Copy link
Copy Markdown
Member

JmPotato commented Nov 4, 2020

/run-all-tests

@0xPoe 0xPoe mentioned this pull request Nov 4, 2020
@Yisaer
Copy link
Copy Markdown
Contributor Author

Yisaer commented Nov 4, 2020

/run-all-tests

@Yisaer
Copy link
Copy Markdown
Contributor Author

Yisaer commented Nov 4, 2020

/merge

@ti-chi-bot
Copy link
Copy Markdown
Member

@Yisaer: you cannot merge your own PR.

Details

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