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

Persist temporary setting to etcd #3131

Merged
merged 39 commits into from
Nov 27, 2020
Merged

Persist temporary setting to etcd #3131

merged 39 commits into from
Nov 27, 2020

Conversation

longfangsong
Copy link
Member

@longfangsong longfangsong commented Nov 2, 2020 •
edited
Loading

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

What problem does this PR solve?

Sometimes lightning and BR want to patch some of the configuation, #3082 support this, but this implemetion store all the temporary setting in memory, this will cause these settings not taking effect when pd master in cluster is down.

What is changed and how it works?

Save the temporary setting to etcd

See this doc for details.

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity

Release note

  • No release note

@longfangsong longfangsong force-pushed the persist-ttl-string branch 5 times, most recently from f9f9e5c to 4da8e77 Compare November 3, 2020 06:32
@Yisaer @longfangsong
tso: Fix Local tso alloactor become alloactor leader without pd leade…
021d688 
…r knowing its dc-location (tikv#3134)

Signed-off-by: longfangsong <longfangsong@icloud.com>
@longfangsong
Merge branch 'master' into persist-ttl-string
04f5c25
@longfangsong
persist to etcd
a677d30 
Signed-off-by: longfangsong <longfangsong@icloud.com>
@disksing disksing requested review from Yisaer and rleungx and removed request for Yisaer November 5, 2020 08:26
@longfangsong
load on boot and change leader
93875ba 
Signed-off-by: longfangsong <longfangsong@icloud.com>
@longfangsong longfangsong marked this pull request as ready for review November 5, 2020 09:38
@longfangsong
Copy link
Member Author

@Yisaer PTAL

@longfangsong
Merge branch 'master' into persist-ttl-string
61809b8 
Signed-off-by: longfangsong <longfangsong@icloud.com>
Yisaer
Yisaer reviewed Nov 6, 2020
View reviewed changes
server/config/persist_options.go Outdated Show resolved Hide resolved
server/config/persist_options.go Outdated Show resolved Hide resolved
server/config/persist_options.go Outdated Show resolved Hide resolved
server/server.go Outdated Show resolved Hide resolved
rleungx
rleungx reviewed Nov 6, 2020
View reviewed changes
server/config/persist_options.go Outdated Show resolved Hide resolved
server/config/persist_options.go Outdated Show resolved Hide resolved
server/config/persist_options.go Outdated Show resolved Hide resolved
server/config/persist_options.go Outdated Show resolved Hide resolved
@longfangsong
Apply several suggestion from code review
220df6f 
Signed-off-by: longfangsong <longfangsong@icloud.com>
@longfangsong
extract getTTLUint and getTTLFloat
2b046be 
Signed-off-by: longfangsong <longfangsong@icloud.com>
@longfangsong
extract EtcdKVPutWithTTL
f66df0f 
Signed-off-by: longfangsong <longfangsong@icloud.com>
@longfangsong
Merge branch 'master' into persist-ttl-string
bf1ae40
@rleungx
Copy link
Member

rleungx commented Nov 25, 2020

There is still a comment which is not resolved

@rleungx
Copy link
Member

rleungx commented Nov 25, 2020

Is there possible to reduce the time more?

@AndreMouche
Copy link
Member

/LGTM

@ti-chi-bot
Copy link
Member

@AndreMouche: adding LGTM is restricted to 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 tidb-community-bots/ti-community-prow 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 27, 2020
@Yisaer
Copy link
Contributor

Yisaer commented Nov 27, 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 tidb-community-bots/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 Nov 27, 2020
@ti-chi-bot
Copy link
Member

Can merge label has been added.

Git tree hash: 9240830

@ti-chi-bot
Copy link
Member

@longfangsong: Your PR has out-of-dated and I have automatically updated it for you.
At the same time I will also trigger all tests for you:

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

@HunDunDM
Copy link
Member

/run-all-tests

@ti-chi-bot ti-chi-bot merged commit 1eeb4c8 into tikv:master Nov 27, 2020
@longfangsong longfangsong mentioned this pull request Nov 27, 2020
Open
@Yisaer Yisaer added this to the v4.0.10 milestone Nov 30, 2020
@Yisaer Yisaer added the needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. label Nov 30, 2020
ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Nov 30, 2020
@longfangsong @ti-srebot
cherry pick tikv#3131 to release-4.0
5f24ab7 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Nov 30, 2020
Merged
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #3228

ti-chi-bot pushed a commit that referenced this pull request Dec 1, 2020
@ti-srebot @longfangsong
Persist temporary setting to etcd (#3131) (#3228)
9b1baad 
* cherry pick #3131 to release-4.0

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

* fix rebasing error

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

Co-authored-by: 龙方淞 <longfangsong@icloud.com>
@rleungx rleungx removed this from the v4.0.10 milestone Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rleungx rleungx rleungx approved these changes

@Yisaer Yisaer Yisaer approved these changes

Assignees
No one assigned
Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@longfangsong @3pointer @rleungx @AndreMouche @ti-chi-bot @Yisaer @HunDunDM @ti-srebot