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

id: fix the id allocator is not monotonic #3305

Merged
merged 1 commit into from
Dec 28, 2020
Merged

Conversation

rleungx
Copy link
Member

@rleungx rleungx commented Dec 25, 2020

What problem does this PR solve?

What is changed and how it works?

This PR tries to make the id monotonic since there are many components that depend on this property. ref: #3303

Check List

Tests

  • Unit test

Related changes

Release note

@rleungx rleungx added the type/bugfix This PR fixes a bug. label Dec 25, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
server/id/id.go Show resolved Hide resolved
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 25, 2020
@@ -1223,6 +1223,10 @@ func (s *Server) campaignLeader() {
log.Error("failed to load persistOptions from etcd", errs.ZapError(err))
return
}
if err := s.idAllocator.Generate(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I think Generate also needs mu.Lock?

@codecov
Copy link

codecov bot commented Dec 25, 2020

Codecov Report

Merging #3305 (ffe37d6) into master (6457a48) will decrease coverage by 0.01%.
The diff coverage is 70.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3305      +/-   ##
==========================================
- Coverage   74.71%   74.69%   -0.02%     
==========================================
  Files         243      243              
  Lines       23207    23210       +3     
==========================================
- Hits        17338    17337       -1     
+ Misses       4292     4289       -3     
- Partials     1577     1584       +7     
Flag Coverage Δ
unittests 74.69% <70.00%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
server/id/id.go 80.00% <57.14%> (+5.00%) ⬆️
server/server.go 72.46% <100.00%> (-1.62%) ⬇️
pkg/tempurl/tempurl.go 45.00% <0.00%> (-15.00%) ⬇️
server/schedule/region_scatterer.go 80.61% <0.00%> (-3.58%) ⬇️
server/tso/tso.go 78.61% <0.00%> (-2.52%) ⬇️
server/config/persist_options.go 91.33% <0.00%> (-0.79%) ⬇️
server/tso/allocator_manager.go 75.45% <0.00%> (-0.41%) ⬇️
client/client.go 64.93% <0.00%> (-0.38%) ⬇️
server/cluster/cluster.go 83.27% <0.00%> (-0.26%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6457a48...ffe37d6. Read the comment docs.

Copy link
Contributor

@nolouch nolouch 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-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 Dec 25, 2020
@Rustin170506
Copy link
Member

/lgtm

@ti-chi-bot
Copy link
Member

@hi-rustin: /lgtm is only allowed for the reviewers in list.

In response to this:

/lgtm

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 ti-community-infra/ti-community-prow repository.

@Yisaer
Copy link
Contributor

Yisaer commented Dec 28, 2020

/merge

@ti-chi-bot
Copy link
Member

@Yisaer: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/ti-community-prow repository.

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

This pull request has been accepted and is ready to merge.

Commit hash: ffe37d6

@ti-chi-bot ti-chi-bot merged commit 65e08e2 into tikv:master Dec 28, 2020
@nolouch
Copy link
Contributor

nolouch commented Dec 28, 2020

/run-cherry-picker

@ti-srebot
Copy link
Contributor

cherry pick to release-5.0-rc in PR #3306

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Dec 28, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@nolouch nolouch added needs-cherry-pick-release-2.1 The PR needs to cherry pick to release-2.1 branch. needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. labels Dec 28, 2020
@nolouch
Copy link
Contributor

nolouch commented Dec 28, 2020

/run-cherry-picker

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Dec 28, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-2.1 in PR #3307

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Dec 28, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #3308

ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Dec 28, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-3.0 in PR #3309

rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Dec 29, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>
ti-chi-bot pushed a commit that referenced this pull request Dec 30, 2020
Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: Ryan Leung <rleungx@gmail.com>
ti-chi-bot added a commit that referenced this pull request Jan 5, 2021
* cherry pick #3305 to release-5.0-rc

Signed-off-by: Ryan Leung <rleungx@gmail.com>

* cherry pick #3322 to release-5.0-rc

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: Ryan Leung <rleungx@gmail.com>
Co-authored-by: ShuNing <nolouch@gmail.com>
Co-authored-by: Ti Prow Robot <71242396+ti-community-prow-bot@users.noreply.github.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 20, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 20, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 25, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 25, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 25, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to ti-srebot/pd that referenced this pull request Jan 25, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
nolouch pushed a commit that referenced this pull request Jul 1, 2021
Signed-off-by: Ryan Leung <rleungx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-2.1 The PR needs to cherry pick to release-2.1 branch. needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. status/can-merge Indicates a PR has been approved by a committer. 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.

7 participants