parser, ddl: update the DDL job's Location structure #31199

zimulala · 2021-12-30T17:22:45Z

What problem does this PR solve?

Issue Number: close #31212

Problem Summary:
The Location in the job is empty after decoding.

What is changed and how it works?

Because the Location object's fields are all lowercase, there are no useful fields after json decoding. So, use Name and Offset instead of Location to represent timezone.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

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

Documentation

Release note

None

ti-chi-bot · 2021-12-30T17:22:46Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

AilinKid
wjhuang2016

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

AilinKid

LGTM

wjhuang2016 · 2021-12-31T02:53:03Z

parser/model/ddl.go

@@ -211,7 +211,13 @@ type DDLReorgMeta struct {
 	SQLMode       mysql.SQLMode                    `json:"sql_mode"`
 	Warnings      map[errors.ErrorID]*terror.Error `json:"warnings"`
 	WarningsCount map[errors.ErrorID]int64         `json:"warnings_count"`
-	Location      *time.Location                   `json:"time_location"`
+	Location      *TimeZone                        `json:"time_zone"`


Would it break the compatibility?

No, I looked at the original PR that added this field that is not on the branch yet.

PR：#29664

wjhuang2016

Need an issue

wjhuang2016 · 2021-12-31T03:19:17Z

parser/model/ddl.go

@@ -211,7 +211,13 @@ type DDLReorgMeta struct {
 	SQLMode       mysql.SQLMode                    `json:"sql_mode"`
 	Warnings      map[errors.ErrorID]*terror.Error `json:"warnings"`
 	WarningsCount map[errors.ErrorID]int64         `json:"warnings_count"`
-	Location      *time.Location                   `json:"time_location"`
+	Location      *TimeZone                        `json:"time_zone"`


Where do you set it at?

I will set it in the next PR（There are a lot of problems involved in this PR, so I will fix this problem first
）.

But I don't know what you want to fix and don't know if it works.

It describes the problems we need to deal with: “The Location in the job is empty after decoding”. It was also written in the original PR of worker to be dealt with.

I mean the problem from the user's perspective.
“The Location in the job is empty after decoding”, Sounds like a problem, but what would happen? I'm still confused.
Besides, why it's empty after decoding? And Why using Name and Offset can fix it?

Because the Location object's fields are all lowercase, there are no useful fields after decoding. I'll add this to the description.

Ok, I get it. Please add an issue for this PR.

@wjhuang2016 Done.

zimulala · 2021-12-31T04:45:16Z

CI failed for：

[2021-12-31T03:36:23.943Z] FAIL: update_test.go:1446: testStatsSuite.TestLogDetailedInfo
[2021-12-31T03:36:23.943Z] 
[2021-12-31T03:36:23.943Z] update_test.go:1507:
[2021-12-31T03:36:23.943Z]     c.Assert(s.hook.results, Equals, t.result)
[2021-12-31T03:36:23.943Z] ... obtained string = ""
[2021-12-31T03:36:23.943Z] ... expected string = "[stats-feedback] test.t, column=a, rangeStr=range: [-inf,8), actual: 8, expected: 8, buckets: {num: 8 lower_bound: 0 upper_bound: 7 repeats: 1 ndv: 0, num: 8 lower_bound: 8 upper_bound: 15 repeats: 1 ndv: 0}[stats-feedback] test.t, column=a, rangeStr=range: [8,15), actual: 8, expected: 7, buckets: {num: 8 lower_bound: 8 upper_bound: 15 repeats: 1 ndv: 0}"

zimulala · 2021-12-31T04:49:38Z

/merge

ti-chi-bot · 2021-12-31T04:49:41Z

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

Commit hash: 00b5d10

zimulala · 2021-12-31T05:13:18Z

/run-check_dev
/run-mysql-test
/run-unit-test

zimulala · 2021-12-31T06:05:07Z

/run-check_dev_2

zimulala · 2021-12-31T06:16:53Z

/run-unit-test

zhouqiang-cl · 2021-12-31T07:17:35Z

/merge

zhouqiang-cl · 2021-12-31T07:24:25Z

/run-unit-tests

zhouqiang-cl · 2021-12-31T07:55:50Z

/merge

zimulala · 2021-12-31T08:02:46Z

/run-mysql-test

sre-bot · 2021-12-31T08:10:07Z

Code Coverage Details: https://codecov.io/github/pingcap/tidb/commit/1446077a431247da335c862084f610342c017468

zimulala · 2021-12-31T08:13:31Z

/run-unit-test

zhouqiang-cl · 2021-12-31T08:20:01Z

/merge

hawkingrei · 2022-01-03T04:04:47Z

/run-unit-test

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

ti-srebot · 2022-01-04T02:33:43Z

cherry pick to release-5.4 in PR #31263

close #31212

parser, ddl: update the DDL job's Location structure

d25e2be

zimulala added the type/bugfix This PR fixes a bug. label Dec 30, 2021

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 30, 2021

AilinKid approved these changes Dec 31, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 31, 2021

wjhuang2016 reviewed Dec 31, 2021

View reviewed changes

Merge branch 'master' into zimuxia/job-tz

6e36167

wjhuang2016 reviewed Dec 31, 2021

View reviewed changes

wjhuang2016 approved these changes Dec 31, 2021

View reviewed changes

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 31, 2021

Merge branch 'master' into zimuxia/job-tz

00b5d10

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 31, 2021

Merge branch 'master' into zimuxia/job-tz

41aa7d4

zimulala added 3 commits December 31, 2021 14:18

Merge branch 'master' into zimuxia/job-tz

0aff664

Merge branch 'master' into zimuxia/job-tz

65c4536

Merge branch 'master' into zimuxia/job-tz

9a25a39

Merge branch 'master' into zimuxia/job-tz

cc6e15f

Merge branch 'master' into zimuxia/job-tz

1883a7e

ti-chi-bot and others added 5 commits December 31, 2021 16:20

Merge branch 'master' into zimuxia/job-tz

ed6aa17

Merge branch 'master' into zimuxia/job-tz

4145f00

Merge branch 'master' into zimuxia/job-tz

e6ce297

Merge branch 'master' into zimuxia/job-tz

0ae28db

Merge branch 'master' into zimuxia/job-tz

8d7bb0d

Merge branch 'master' into zimuxia/job-tz

1446077

ti-chi-bot merged commit e50e03c into pingcap:master Jan 3, 2022

zimulala added the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Jan 4, 2022

ti-srebot mentioned this pull request Jan 4, 2022

parser, ddl: update the DDL job's Location structure (#31199) #31263

Merged

12 tasks

ti-srebot pushed a commit to ti-srebot/tidb that referenced this pull request Jan 4, 2022

cherry pick pingcap#31199 to release-5.4

85021c7

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

zimulala deleted the zimuxia/job-tz branch January 4, 2022 02:34

ti-chi-bot pushed a commit that referenced this pull request Jan 6, 2022

parser, ddl: update the DDL job's Location structure (#31199) (#31263)

804a17a

close #31212

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parser, ddl: update the DDL job's Location structure #31199

parser, ddl: update the DDL job's Location structure #31199

zimulala commented Dec 30, 2021 •

edited

Loading

ti-chi-bot commented Dec 30, 2021 •

edited

Loading

AilinKid left a comment

wjhuang2016 Dec 31, 2021

zimulala Dec 31, 2021

wjhuang2016 Dec 31, 2021

zimulala Dec 31, 2021

wjhuang2016 left a comment

wjhuang2016 Dec 31, 2021

zimulala Dec 31, 2021

wjhuang2016 Dec 31, 2021

zimulala Dec 31, 2021

wjhuang2016 Dec 31, 2021

zimulala Dec 31, 2021 •

edited

Loading

wjhuang2016 Dec 31, 2021

zimulala Dec 31, 2021

zimulala commented Dec 31, 2021

zimulala commented Dec 31, 2021

ti-chi-bot commented Dec 31, 2021

zimulala commented Dec 31, 2021

zimulala commented Dec 31, 2021

zimulala commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

zimulala commented Dec 31, 2021

sre-bot commented Dec 31, 2021 •

edited

Loading

zimulala commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

hawkingrei commented Jan 3, 2022

ti-srebot commented Jan 4, 2022

parser, ddl: update the DDL job's Location structure #31199

parser, ddl: update the DDL job's Location structure #31199

Conversation

zimulala commented Dec 30, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Dec 30, 2021 • edited Loading

AilinKid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wjhuang2016 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala Dec 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala commented Dec 31, 2021

zimulala commented Dec 31, 2021

ti-chi-bot commented Dec 31, 2021

zimulala commented Dec 31, 2021

zimulala commented Dec 31, 2021

zimulala commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

zimulala commented Dec 31, 2021

sre-bot commented Dec 31, 2021 • edited Loading

zimulala commented Dec 31, 2021

zhouqiang-cl commented Dec 31, 2021

hawkingrei commented Jan 3, 2022

ti-srebot commented Jan 4, 2022

zimulala commented Dec 30, 2021 •

edited

Loading

ti-chi-bot commented Dec 30, 2021 •

edited

Loading

zimulala Dec 31, 2021 •

edited

Loading

sre-bot commented Dec 31, 2021 •

edited

Loading