lightning: skip restore table when source files are empty #28189

glorv · 2021-09-18T03:29:02Z

What problem does this PR solve?

Issue Number: close #28144

Problem Summary:

What is changed and how it works?

Skip restore table when there are no source files.

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

Fix the bug that lightning may not clean up metadata schema when some of the import contains no source files.

ti-chi-bot · 2021-09-18T03:29:04Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

gozssky
kennytm

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.

sleepymole · 2021-09-18T03:45:45Z

There are at least four special judgments(include this PR) for empty source table 😂, can we unify them in one place?

glorv · 2021-09-18T06:06:58Z

/run-all-tests

glorv · 2021-09-18T06:07:12Z

/run-integration-br-tests

sleepymole · 2021-09-18T06:27:51Z

br/pkg/lightning/restore/restore.go

@@ -1349,6 +1349,9 @@ func (rc *Controller) restoreTables(ctx context.Context) error {
 			if err != nil {
 				return errors.Trace(err)
 			}
+			if cp.Status < checkpoints.CheckpointStatusAllWritten && len(tableMeta.DataFiles) == 0 {


Why cp.Status < checkpoints.CheckpointStatusAllWritten is needed here?

If cp.Status >= CheckpointStatusAllWritten then the restore process has no relation with the source files anymore, so we may allow it to continue even if we find there are no source files. Maybe there are situations that the source files are corrupted, the import phase can still go on.😅

Comparing statue is wired. Status is not a clear name, Rename it to stage or something similar would be more clear.

Little-Wallace

LGTM. But I think we shall not decide checksum and meta-table in one condition code.

glorv · 2021-10-19T12:00:24Z

LGTM. But I think we shall not decide checksum and meta-table in one condition code.

I thought about this, but it's not easy to do so. In theory the management of metadata should be decouple, but because only after checksum can we know whether the metadata can be cleaned up. Another option is to always do check and cleanup on demand after restore table, but this kind of approach can't take advantage of the information after checksum. Not sure if we need to move this cleanup logic after restore table, the logic will be more complex than the current implement.

glorv · 2021-10-19T12:00:40Z

/hold

glorv · 2022-02-15T07:28:42Z

/run-integration-br-test

kennytm · 2022-02-15T08:01:30Z

the new test failed.

https://ci.pingcap.net/blue/organizations/jenkins/br_ghpr_unit_and_integration_test/detail/br_ghpr_unit_and_integration_test/10068/pipeline/102

[2022-02-15T07:45:40.787Z] *************************** 1. row ***************************

[2022-02-15T07:45:40.787Z] Database: lightning_metadata

[2022-02-15T07:45:40.787Z] TEST FAILED: OUTPUT CONTAINS 'Database: lightning_metadata'

[2022-02-15T07:45:40.787Z] ____________________________________

[2022-02-15T07:45:40.787Z] [Tue Feb 15 15:45:39 CST 2022] Executing SQL: SHOW DATABASES like 'lightning_metadata';

[2022-02-15T07:45:40.787Z] *************************** 1. row ***************************

[2022-02-15T07:45:40.787Z] Database: lightning_metadata

[2022-02-15T07:45:40.787Z] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

glorv · 2022-02-15T09:10:34Z

/run-integration-br-test

glorv · 2022-02-15T09:21:11Z

the new test failed.

https://ci.pingcap.net/blue/organizations/jenkins/br_ghpr_unit_and_integration_test/detail/br_ghpr_unit_and_integration_test/10068/pipeline/102

[2022-02-15T07:45:40.787Z] *************************** 1. row ***************************

[2022-02-15T07:45:40.787Z] Database: lightning_metadata

[2022-02-15T07:45:40.787Z] TEST FAILED: OUTPUT CONTAINS 'Database: lightning_metadata'

[2022-02-15T07:45:40.787Z] ____________________________________

[2022-02-15T07:45:40.787Z] [Tue Feb 15 15:45:39 CST 2022] Executing SQL: SHOW DATABASES like 'lightning_metadata';

[2022-02-15T07:45:40.787Z] *************************** 1. row ***************************

[2022-02-15T07:45:40.787Z] Database: lightning_metadata

[2022-02-15T07:45:40.787Z] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Fixed. Since we don't support incremental restore for importer backend, I just from the case for importer in lightning_incremental.

glorv · 2022-02-16T02:02:44Z

/merge

ti-chi-bot · 2022-02-16T02:02:47Z

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

Commit hash: d4e8eef

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2022-02-16T02:21:42Z

cherry pick to release-5.2 in PR #32388

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2022-02-16T02:23:17Z

cherry pick to release-5.3 in PR #32389

Signed-off-by: ti-srebot <[email protected]>

ti-srebot · 2022-02-16T02:25:11Z

cherry pick to release-5.4 in PR #32390

…32389) close #28144

…32388) close #28144

…32390) close #28144

skip restore table when source files are empty

241b427

glorv added needs-cherry-pick-release-5.2 component/lightning This issue is related to Lightning of TiDB. labels Sep 18, 2021

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 18, 2021

glorv requested review from Little-Wallace, kennytm and sleepymole September 18, 2021 03:29

remove useless code

8821696

ti-chi-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 18, 2021

kennytm changed the title ~~br: skip restore table when source files are empty~~ lightning: skip restore table when source files are empty Sep 18, 2021

sleepymole reviewed Sep 18, 2021

View reviewed changes

sleepymole approved these changes Sep 18, 2021

View reviewed changes

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

Little-Wallace reviewed Oct 17, 2021

View reviewed changes

Merge branch 'master' of ssh://github.com/pingcap/tidb into fix-meta

f8a745f

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 19, 2021

glorv added 2 commits February 15, 2022 14:43

Merge branch 'master' of ssh://github.com/pingcap/tidb into fix-meta

e05154c

add a integration test

730e1cb

add clean up

d1dc557

remove import backend from integration test 'lightning_incremental'

70a2376

ti-chi-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 Feb 15, 2022

Merge branch 'master' into fix-meta

d4e8eef

kennytm approved these changes Feb 15, 2022

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 Feb 15, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 16, 2022

Merge branch 'master' into fix-meta

7281719

ti-chi-bot merged commit 706f8a9 into pingcap:master Feb 16, 2022

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

cherry pick pingcap#28189 to release-5.2

cabe910

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Feb 16, 2022

lightning: skip restore table when source files are empty (#28189) #32388

Merged

12 tasks

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

cherry pick pingcap#28189 to release-5.3

5bbd689

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Feb 16, 2022

lightning: skip restore table when source files are empty (#28189) #32389

Merged

12 tasks

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

cherry pick pingcap#28189 to release-5.4

eb03b3f

Signed-off-by: ti-srebot <[email protected]>

ti-srebot mentioned this pull request Feb 16, 2022

lightning: skip restore table when source files are empty (#28189) #32390

Merged

12 tasks

glorv deleted the fix-meta branch February 16, 2022 02:26

ti-chi-bot pushed a commit that referenced this pull request Feb 18, 2022

lightning: skip restore table when source files are empty (#28189) (#…

0063773

…32389) close #28144

ti-chi-bot pushed a commit that referenced this pull request Apr 13, 2022

lightning: skip restore table when source files are empty (#28189) (#…

5a6e55c

…32388) close #28144

ti-chi-bot pushed a commit that referenced this pull request Apr 24, 2022

lightning: skip restore table when source files are empty (#28189) (#…

e29bbda

…32390) close #28144

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lightning: skip restore table when source files are empty #28189

lightning: skip restore table when source files are empty #28189

glorv commented Sep 18, 2021 •

edited

Loading

ti-chi-bot commented Sep 18, 2021 •

edited

Loading

sleepymole commented Sep 18, 2021

glorv commented Sep 18, 2021

glorv commented Sep 18, 2021

sleepymole Sep 18, 2021

glorv Sep 18, 2021

ngaut Sep 21, 2021

Little-Wallace left a comment

glorv commented Oct 19, 2021

glorv commented Oct 19, 2021

glorv commented Feb 15, 2022

kennytm commented Feb 15, 2022 •

edited

Loading

glorv commented Feb 15, 2022

glorv commented Feb 15, 2022

glorv commented Feb 16, 2022

ti-chi-bot commented Feb 16, 2022

ti-srebot commented Feb 16, 2022

ti-srebot commented Feb 16, 2022

ti-srebot commented Feb 16, 2022

lightning: skip restore table when source files are empty #28189

lightning: skip restore table when source files are empty #28189

Conversation

glorv commented Sep 18, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Sep 18, 2021 • edited Loading

sleepymole commented Sep 18, 2021

glorv commented Sep 18, 2021

glorv commented Sep 18, 2021

sleepymole Sep 18, 2021

Choose a reason for hiding this comment

glorv Sep 18, 2021

Choose a reason for hiding this comment

ngaut Sep 21, 2021

Choose a reason for hiding this comment

Little-Wallace left a comment

Choose a reason for hiding this comment

glorv commented Oct 19, 2021

glorv commented Oct 19, 2021

glorv commented Feb 15, 2022

kennytm commented Feb 15, 2022 • edited Loading

glorv commented Feb 15, 2022

glorv commented Feb 15, 2022

glorv commented Feb 16, 2022

ti-chi-bot commented Feb 16, 2022

ti-srebot commented Feb 16, 2022

ti-srebot commented Feb 16, 2022

ti-srebot commented Feb 16, 2022

glorv commented Sep 18, 2021 •

edited

Loading

ti-chi-bot commented Sep 18, 2021 •

edited

Loading

kennytm commented Feb 15, 2022 •

edited

Loading