ddl: make sure put key into ETCD monotonously #52381

wjhuang2016 · 2024-04-07T08:36:47Z

What problem does this PR solve?

Issue Number: close #47060 and close #52335

Problem Summary:

What changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

codecov · 2024-04-07T09:13:50Z

Codecov Report

Merging #52381 (8e02ae4) into master (dfb7955) will decrease coverage by 0.5688%.
Report is 83 commits behind head on master.
The diff coverage is 75.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #52381        +/-   ##
================================================
- Coverage   70.7107%   70.1420%   -0.5688%     
================================================
  Files          1492       1545        +53     
  Lines        440257     460239     +19982     
================================================
+ Hits         311309     322821     +11512     
- Misses       109448     116976      +7528     
- Partials      19500      20442       +942

Flag	Coverage Δ
integration	`27.4402% <61.1111%> (?)`

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

Components	Coverage Δ
dumpling	`53.9957% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`27.4756% <ø> (-18.4100%)`	⬇️

pkg/ddl/util/util.go

Co-authored-by: tangenta <tangenta@126.com>

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot · 2024-04-09T13:40:26Z

@tangenta: Your lgtm message is repeated, so it is ignored.

In response to this:

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 kubernetes/test-infra repository.

zimulala · 2024-04-09T13:57:54Z

pkg/ddl/util/util.go

+		resp, err = etcdCli.Get(childCtx, key)
+		if err != nil {
+			cancel()
+			logutil.BgLogger().Warn("error when getting key revision", zap.String("category", "ddl"), zap.Error(err))


I see that the log or error is usually used like this: failed to get key revision or get key revision failed. Also like line334, line332.

zimulala · 2024-04-09T13:58:56Z

pkg/ddl/util/util.go

+		cancel()
+
+		if err != nil {
+			logutil.BgLogger().Warn("error when performing compare-and-swap", zap.String("category", "ddl"), zap.Error(err))


zimulala · 2024-04-09T14:04:32Z

pkg/ddl/util/util.go

+		cancel()
+
+		if err != nil {
+			logutil.BgLogger().Warn("error when performing compare-and-swap", zap.String("category", "ddl"), zap.Error(err))


Do we need to sleep like line335? Maybe we can return err == nil && txnResp.Succeeded first. Then remaining two cases do the same(maybe the log is different).

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot · 2024-04-09T14:21:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tangenta, zimulala

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/ddl/OWNERS~~ [tangenta,zimulala]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-04-09T14:21:51Z

[LGTM Timeline notifier]

Timeline:

2024-04-08 08:10:51.059933988 +0000 UTC m=+863512.587474527: ☑️ agreed by tangenta.
2024-04-09 14:21:49.820940983 +0000 UTC m=+972171.348481529: ☑️ agreed by zimulala.

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot · 2024-04-10T10:55:38Z

In response to a cherrypick label: new pull request created to branch release-7.1: #52480.

ti-chi-bot · 2024-04-10T10:55:41Z

In response to a cherrypick label: new pull request created to branch release-6.5: #52481.

ti-chi-bot · 2024-04-10T10:55:42Z

In response to a cherrypick label: new pull request created to branch release-7.5: #52482.

ti-chi-bot · 2024-04-10T10:55:43Z

In response to a cherrypick label: new pull request created to branch release-8.1: #52483.

close pingcap#47060, close pingcap#52335

close #47060, close #52335

done

dfd6752

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot bot added do-not-merge/needs-tests-checked do-not-merge/needs-triage-completed release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed do-not-merge/needs-triage-completed labels Apr 7, 2024

ti-chi-bot bot added do-not-merge/needs-triage-completed and removed do-not-merge/needs-triage-completed do-not-merge/needs-tests-checked labels Apr 8, 2024

tangenta approved these changes Apr 8, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 8, 2024

tangenta reviewed Apr 8, 2024

View reviewed changes

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

tangenta reviewed Apr 8, 2024

View reviewed changes

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

wjhuang2016 and others added 3 commits April 9, 2024 16:07

Update pkg/ddl/util/util.go

bf78b04

Co-authored-by: tangenta <tangenta@126.com>

Update pkg/ddl/util/util.go

114684b

Co-authored-by: tangenta <tangenta@126.com>

add test

26e0fee

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 9, 2024

tangenta approved these changes Apr 9, 2024

View reviewed changes

zimulala reviewed Apr 9, 2024

View reviewed changes

refine

fe42170

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

zimulala approved these changes Apr 9, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Apr 9, 2024

fix

8e02ae4

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

ti-chi-bot bot merged commit 312b69f into pingcap:master Apr 9, 2024
22 of 23 checks passed

ti-chi-bot mentioned this pull request Apr 10, 2024

ddl: make sure put key into ETCD monotonously (#52381) #52480

Merged

13 tasks

ti-chi-bot mentioned this pull request Apr 10, 2024

ddl: make sure put key into ETCD monotonously (#52381) #52481

Merged

13 tasks

ti-chi-bot mentioned this pull request Apr 10, 2024

ddl: make sure put key into ETCD monotonously (#52381) #52482

Merged

13 tasks

ti-chi-bot mentioned this pull request Apr 10, 2024

ddl: make sure put key into ETCD monotonously (#52381) #52483

Merged

13 tasks

3AceShowHand pushed a commit to 3AceShowHand/tidb that referenced this pull request Apr 16, 2024

ddl: make sure put key into ETCD monotonously (pingcap#52381)

66f93c3

close pingcap#47060, close pingcap#52335

ti-chi-bot bot pushed a commit that referenced this pull request Apr 16, 2024

ddl: make sure put key into ETCD monotonously (#52381) (#52480)

43222f7

close #47060, close #52335

ti-chi-bot bot pushed a commit that referenced this pull request May 9, 2024

ddl: make sure put key into ETCD monotonously (#52381) (#52483)

64ca76e

close #47060, close #52335

ti-chi-bot bot pushed a commit that referenced this pull request May 28, 2024

ddl: make sure put key into ETCD monotonously (#52381) (#52482)

389e73c

close #47060, close #52335

ti-chi-bot removed the needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. label Jun 3, 2024

ti-chi-bot bot pushed a commit that referenced this pull request Jun 5, 2024

ddl: make sure put key into ETCD monotonously (#52381) (#52481)

60e6e0b

close #47060, close #52335

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: make sure put key into ETCD monotonously #52381

ddl: make sure put key into ETCD monotonously #52381

wjhuang2016 commented Apr 7, 2024 •

edited

Loading

codecov bot commented Apr 7, 2024 •

edited

Loading

ti-chi-bot bot commented Apr 9, 2024

zimulala Apr 9, 2024

zimulala Apr 9, 2024

zimulala Apr 9, 2024

ti-chi-bot bot commented Apr 9, 2024

ti-chi-bot bot commented Apr 9, 2024

ti-chi-bot commented Apr 10, 2024

ti-chi-bot commented Apr 10, 2024

ti-chi-bot commented Apr 10, 2024

ti-chi-bot commented Apr 10, 2024

ddl: make sure put key into ETCD monotonously #52381

ddl: make sure put key into ETCD monotonously #52381

Conversation

wjhuang2016 commented Apr 7, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

codecov bot commented Apr 7, 2024 • edited Loading

Codecov Report

ti-chi-bot bot commented Apr 9, 2024

zimulala Apr 9, 2024

Choose a reason for hiding this comment

zimulala Apr 9, 2024

Choose a reason for hiding this comment

zimulala Apr 9, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Apr 9, 2024

ti-chi-bot bot commented Apr 9, 2024

[LGTM Timeline notifier]

ti-chi-bot commented Apr 10, 2024

ti-chi-bot commented Apr 10, 2024

ti-chi-bot commented Apr 10, 2024

ti-chi-bot commented Apr 10, 2024

wjhuang2016 commented Apr 7, 2024 •

edited

Loading

codecov bot commented Apr 7, 2024 •

edited

Loading