*: don't save table IDs in schema diff of FLASHBACK DATABASE #54665

lance6716 · 2024-07-16T08:42:50Z

What problem does this PR solve?

Issue Number: close #54415

Problem Summary:

What changed and how does it work?

as title. Follow up of #54439

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: lance6716 <lance6716@gmail.com>

tiprow · 2024-07-16T08:43:15Z

Hi @lance6716. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

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-sigs/prow repository.

Signed-off-by: lance6716 <lance6716@gmail.com>

codecov · 2024-07-16T09:52:45Z

Codecov Report

Attention: Patch coverage is 57.14286% with 9 lines in your changes missing coverage. Please review.

Project coverage is 56.3531%. Comparing base (bbc13a1) to head (8f5520c).
Report is 793 commits behind head on master.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #54665         +/-   ##
=================================================
- Coverage   72.7514%   56.3531%   -16.3984%     
=================================================
  Files          1551       1672        +121     
  Lines        436702     615251     +178549     
=================================================
+ Hits         317707     346713      +29006     
- Misses        99433     245121     +145688     
- Partials      19562      23417       +3855

Flag	Coverage Δ
integration	`37.2795% <47.6190%> (?)`
unit	`71.7946% <57.1428%> (+0.0140%)`	⬆️

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

Components	Coverage Δ
dumpling	`52.9656% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`52.5535% <ø> (+6.6910%)`	⬆️

lance6716 · 2024-07-17T12:40:17Z

/cc @tangenta

D3Hunter · 2024-07-18T02:37:33Z

pkg/parser/model/ddl.go

@@ -1199,6 +1199,9 @@ type SchemaDiff struct {
 	OldSchemaID int64 `json:"old_schema_id"`
 	// RegenerateSchemaMap means whether to rebuild the schema map when applying to the schema diff.
 	RegenerateSchemaMap bool `json:"regenerate_schema_map"`
+	// RecoverSnapshotTS is set when the diff is too large to be saved in SchemaDiff.
+	// infoschema should use this TS to read meta directly.
+	RecoverSnapshotTS uint64 `json:"recover_snapshot_ts,omitempty"`


what if: after flashback schema job to done, and before reload, GC remove all those keys

I remember it will fallback to full load

tidb/pkg/domain/domain.go

Line 345 in e858f55

// We can fall back to full load, don't need to return the error.

we can reload this schema only if we use current ts

D3Hunter · 2024-07-18T02:39:16Z

pkg/infoschema/infoschema_v2.go

@@ -871,6 +871,25 @@ func applyDropSchema(b *Builder, diff *model.SchemaDiff) []int64 {
 }

 func applyRecoverSchema(b *Builder, m *meta.Meta, diff *model.SchemaDiff) ([]int64, error) {
+	if snapTS := diff.RecoverSnapshotTS; snapTS > 0 {


i prefer reload from now, there is no difference.

Just in case the database is relatively small compared with full load.

why full load? i mean load using current ts, not this saved one

D3Hunter

rest lgtm

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot · 2024-07-18T05:06:34Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, wjhuang2016

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:

~~OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/ddl/OWNERS~~ [wjhuang2016]
~~pkg/domain/OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/infoschema/OWNERS~~ [D3Hunter,wjhuang2016]
~~pkg/parser/OWNERS~~ [D3Hunter]

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-07-18T05:06:37Z

[LGTM Timeline notifier]

Timeline:

2024-07-17 09:02:53.279632118 +0000 UTC m=+430995.270573588: ☑️ agreed by wjhuang2016.
2024-07-18 05:06:36.833028168 +0000 UTC m=+503218.823969636: ☑️ agreed by D3Hunter.

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-07-18T07:53:21Z

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-07-26T07:46:10Z

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2024-10-28T08:22:33Z

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

*: don't save table IDs in schema diff of FLASHBACK DATABASE

8513605

Signed-off-by: lance6716 <lance6716@gmail.com>

ti-chi-bot bot added the release-note-none Denotes a PR that doesn't merit a release note. label Jul 16, 2024

ti-chi-bot added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Jul 16, 2024

ti-chi-bot bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 16, 2024

lance6716 added 3 commits July 16, 2024 16:50

fix bazel

f28430b

Signed-off-by: lance6716 <lance6716@gmail.com>

fix UT

bf73d61

Signed-off-by: lance6716 <lance6716@gmail.com>

fix UT

5488ec2

Signed-off-by: lance6716 <lance6716@gmail.com>

wjhuang2016 approved these changes Jul 17, 2024

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Jul 17, 2024

ti-chi-bot bot requested a review from tangenta July 17, 2024 12:40

D3Hunter reviewed Jul 18, 2024

View reviewed changes

address comment

8f5520c

Signed-off-by: lance6716 <lance6716@gmail.com>

D3Hunter approved these changes Jul 18, 2024

View reviewed changes

ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jul 18, 2024

ti-chi-bot bot merged commit 320d0f4 into pingcap:master Jul 18, 2024
23 checks passed

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 18, 2024

This is an automated cherry-pick of pingcap#54665

13840e3

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jul 18, 2024

*: don't save table IDs in schema diff of FLASHBACK DATABASE (#54665) #54737

Closed

13 tasks

ti-chi-bot added the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Jul 26, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jul 26, 2024

This is an automated cherry-pick of pingcap#54665

244beb6

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jul 26, 2024

*: don't save table IDs in schema diff of FLASHBACK DATABASE (#54665) #54953

Closed

13 tasks

ti-chi-bot removed the needs-cherry-pick-release-8.1 Should cherry pick this PR to release-8.1 branch. label Jul 31, 2024

ti-chi-bot bot removed the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Sep 20, 2024

ti-chi-bot bot added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Oct 28, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Oct 28, 2024

This is an automated cherry-pick of pingcap#54665

3b808b6

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Oct 28, 2024

*: don't save table IDs in schema diff of FLASHBACK DATABASE (#54665) #56905

Closed

13 tasks

ti-chi-bot bot removed the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Nov 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: don't save table IDs in schema diff of FLASHBACK DATABASE #54665

*: don't save table IDs in schema diff of FLASHBACK DATABASE #54665

lance6716 commented Jul 16, 2024

tiprow bot commented Jul 16, 2024

codecov bot commented Jul 16, 2024 •

edited

Loading

lance6716 commented Jul 17, 2024

D3Hunter Jul 18, 2024

lance6716 Jul 18, 2024 •

edited

Loading

D3Hunter Jul 18, 2024

D3Hunter Jul 18, 2024

lance6716 Jul 18, 2024

D3Hunter Jul 18, 2024

lance6716 Jul 18, 2024

D3Hunter left a comment

ti-chi-bot bot commented Jul 18, 2024

ti-chi-bot bot commented Jul 18, 2024

ti-chi-bot commented Jul 18, 2024

ti-chi-bot commented Jul 26, 2024

ti-chi-bot commented Oct 28, 2024

*: don't save table IDs in schema diff of FLASHBACK DATABASE #54665

*: don't save table IDs in schema diff of FLASHBACK DATABASE #54665

Conversation

lance6716 commented Jul 16, 2024

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

tiprow bot commented Jul 16, 2024

codecov bot commented Jul 16, 2024 • edited Loading

Codecov Report

lance6716 commented Jul 17, 2024

D3Hunter Jul 18, 2024

Choose a reason for hiding this comment

lance6716 Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

D3Hunter Jul 18, 2024

Choose a reason for hiding this comment

D3Hunter Jul 18, 2024

Choose a reason for hiding this comment

lance6716 Jul 18, 2024

Choose a reason for hiding this comment

D3Hunter Jul 18, 2024

Choose a reason for hiding this comment

lance6716 Jul 18, 2024

Choose a reason for hiding this comment

D3Hunter left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jul 18, 2024

ti-chi-bot bot commented Jul 18, 2024

[LGTM Timeline notifier]

ti-chi-bot commented Jul 18, 2024

ti-chi-bot commented Jul 26, 2024

ti-chi-bot commented Oct 28, 2024

codecov bot commented Jul 16, 2024 •

edited

Loading

lance6716 Jul 18, 2024 •

edited

Loading