ddl: Remove expired keys on PD #10406

zimulala · 2019-05-09T09:11:27Z

What problem does this PR solve?

We encounter the problem of etcd-io/etcd#9935. And @nolouch pull a PR to fix it.
Just in case, let's do a protective function.

What is changed and how it works?

Regularly deleted expired paths on etcd.

Check List

Tests

Unit test

Related changes

Need to cherry-pick to the release branch

zhouqiang-cl · 2019-05-09T14:33:57Z

/run-all-tests

ddl/ddl_worker.go

ddl/syncer.go

codecov · 2019-05-30T05:59:12Z

Codecov Report

Merging #10406 into master will decrease coverage by 0.4101%.
The diff coverage is 8.4337%.

@@               Coverage Diff                @@
##             master     #10406        +/-   ##
================================================
- Coverage   78.1549%   77.7447%   -0.4102%     
================================================
  Files           413        410         -3     
  Lines         87498      84443      -3055     
================================================
- Hits          68384      65650      -2734     
+ Misses        13971      13886        -85     
+ Partials       5143       4907       -236

zimulala · 2019-05-30T06:12:46Z

PTAL @bb7133 @winkyao

ddl/syncer.go

winkyao · 2019-05-30T08:54:55Z

ddl/syncer.go

+
+		select {
+		case <-s.notifyCleanExpiredPathsCh:
+			for i := 0; i < opDefaultRetryCnt; i++ {


We can extract these codes into a func, to make it more readable.

I think it's OK. Here only get leases and do doCleanExpirePaths.

but the break in line 433 is not very obvious.

It does a retry, I think it's OK.

ddl/syncer.go

zimulala · 2019-05-31T05:52:55Z

PTAL @winkyao @bb7133 @crazycs520

codecov · 2019-05-31T05:57:21Z

Codecov Report

Merging #10406 into master will increase coverage by 0.0365%.
The diff coverage is 78.6516%.

@@               Coverage Diff                @@
##             master     #10406        +/-   ##
================================================
+ Coverage   80.8732%   80.9097%   +0.0365%     
================================================
  Files           419        419                
  Lines         88661      88684        +23     
================================================
+ Hits          71703      71754        +51     
+ Misses        11731      11700        -31     
- Partials       5227       5230         +3

ddl/syncer.go

winkyao

rest LGTM

winkyao · 2019-06-04T03:50:02Z

@zimulala Please address comments

crazycs520

LGTM

crazycs520 · 2019-06-18T07:44:56Z

/run-all-tests

winkyao

LGTM

ddl/ddl.go

lonng · 2019-06-19T11:50:29Z

ddl/util/syncer.go

-	// WaitTimeWhenErrorOccured is waiting interval when processing DDL jobs encounter errors.
-	WaitTimeWhenErrorOccured = 1 * time.Second
+	// ddlLogCtx uses for log.
+	ddlLogCtx = context.Background()


Lots of logutil.Logger(context.Background()) code snippets exist in this repo, I think we can provide a utility function logutil.BgLogger() to improve it.

We can refactor it in other PR.

ddl/util/syncer.go

zimulala · 2019-06-19T16:48:40Z

PTAL @lonng

lonng

LGTM

zimulala · 2019-06-20T02:05:20Z

/run-unit-test

zimulala · 2019-06-20T05:36:33Z

/run-integration-ddl-test tidb-test=pr/835

zimulala · 2019-06-20T05:44:27Z

/run-common-test tidb-test=pr/835

ddl: remove expired keys on etcd

3bfae8b

zimulala added status/WIP labels May 9, 2019

ddl: tiny update

4ceaff8

zimulala force-pushed the rm-etcd-keys branch from d99ef8e to 4ceaff8 Compare May 9, 2019 09:16

winkyao reviewed May 29, 2019

View reviewed changes

ddl/ddl_worker.go Outdated Show resolved Hide resolved

bb7133 reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

bb7133 reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

bb7133 reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

bb7133 reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

winkyao reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Show resolved Hide resolved

ddl: add comments and tiny update

6e932a2

zimulala force-pushed the rm-etcd-keys branch from 46d4850 to 6e932a2 Compare May 30, 2019 05:39

zimulala force-pushed the rm-etcd-keys branch from 484f75f to 07544d0 Compare May 30, 2019 06:03

zimulala added status/DNM and removed status/WIP labels May 30, 2019

ddl: address comments

268ea04

zimulala force-pushed the rm-etcd-keys branch from 07544d0 to 268ea04 Compare May 30, 2019 06:11

winkyao reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

winkyao reviewed May 30, 2019

View reviewed changes

crazycs520 reviewed May 30, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

ddl: address comments

f8a6b2b

winkyao reviewed May 31, 2019

View reviewed changes

ddl/syncer.go Outdated Show resolved Hide resolved

winkyao reviewed May 31, 2019

View reviewed changes

crazycs520 reviewed Jun 18, 2019

View reviewed changes

crazycs520 added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 18, 2019

winkyao reviewed Jun 18, 2019

View reviewed changes

winkyao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jun 18, 2019

lonng reviewed Jun 19, 2019

View reviewed changes

ddl/ddl.go Show resolved Hide resolved

lonng reviewed Jun 19, 2019

View reviewed changes

ddl/util/syncer.go Outdated Show resolved Hide resolved

ddl: remove checkCleanJobInterval and update a log

1a62b70

zimulala force-pushed the rm-etcd-keys branch from 0a180aa to 1a62b70 Compare June 19, 2019 16:47

lonng approved these changes Jun 19, 2019

View reviewed changes

zimulala added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jun 20, 2019

winkyao and others added 3 commits June 20, 2019 10:37

Merge branch 'master' into rm-etcd-keys

5b599d7

Merge branch 'master' into rm-etcd-keys

a475aa8

Merge branch 'master' into rm-etcd-keys

44a0394

zimulala merged commit d23b894 into pingcap:master Jun 20, 2019

zimulala deleted the rm-etcd-keys branch June 20, 2019 05:48

zimulala added a commit to zimulala/tidb that referenced this pull request Jun 28, 2019

ddl: Remove expired keys on PD (pingcap#10406)

12dca08

zimulala mentioned this pull request Jul 1, 2019

ddl: remove expired keys on PD (#10406) #11014

Merged

zimulala added a commit to zimulala/tidb that referenced this pull request Jul 2, 2019

ddl: Remove expired keys on PD (pingcap#10406)

014f870

zimulala mentioned this pull request Jul 2, 2019

ddl: Remove expired keys on PD (#10406) #11023

Merged

zimulala added a commit that referenced this pull request Jul 4, 2019

ddl: Remove expired keys on PD (#10406) (#11023)

842acd8

zimulala added a commit that referenced this pull request Jul 5, 2019

ddl: Remove expired keys on PD (#10406) (#11014)

8493e45

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: Remove expired keys on PD #10406

ddl: Remove expired keys on PD #10406

zimulala commented May 9, 2019 •

edited

Loading

zhouqiang-cl commented May 9, 2019

codecov bot commented May 30, 2019 •

edited

Loading

zimulala commented May 30, 2019

winkyao May 30, 2019

zimulala May 31, 2019

winkyao May 31, 2019

zimulala Jun 6, 2019

zimulala commented May 31, 2019

codecov bot commented May 31, 2019 •

edited

Loading

winkyao left a comment

winkyao commented Jun 4, 2019

crazycs520 left a comment

crazycs520 commented Jun 18, 2019

winkyao left a comment

lonng Jun 19, 2019

winkyao Jun 19, 2019

lonng Jun 19, 2019

zimulala commented Jun 19, 2019

lonng left a comment

zimulala commented Jun 20, 2019

zimulala commented Jun 20, 2019

zimulala commented Jun 20, 2019

ddl: Remove expired keys on PD #10406

ddl: Remove expired keys on PD #10406

Conversation

zimulala commented May 9, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

zhouqiang-cl commented May 9, 2019

codecov bot commented May 30, 2019 • edited Loading

Codecov Report

zimulala commented May 30, 2019

winkyao May 30, 2019

Choose a reason for hiding this comment

zimulala May 31, 2019

Choose a reason for hiding this comment

winkyao May 31, 2019

Choose a reason for hiding this comment

zimulala Jun 6, 2019

Choose a reason for hiding this comment

zimulala commented May 31, 2019

codecov bot commented May 31, 2019 • edited Loading

Codecov Report

winkyao left a comment

Choose a reason for hiding this comment

winkyao commented Jun 4, 2019

crazycs520 left a comment

Choose a reason for hiding this comment

crazycs520 commented Jun 18, 2019

winkyao left a comment

Choose a reason for hiding this comment

lonng Jun 19, 2019

Choose a reason for hiding this comment

winkyao Jun 19, 2019

Choose a reason for hiding this comment

lonng Jun 19, 2019

Choose a reason for hiding this comment

zimulala commented Jun 19, 2019

lonng left a comment

Choose a reason for hiding this comment

zimulala commented Jun 20, 2019

zimulala commented Jun 20, 2019

zimulala commented Jun 20, 2019

zimulala commented May 9, 2019 •

edited

Loading

codecov bot commented May 30, 2019 •

edited

Loading

codecov bot commented May 31, 2019 •

edited

Loading