lightning: save status checkpoint synchronously

[sleepymole]

What problem does this PR solve?

Close pingcap/br#1372.
Same as pingcap/br#1304.

We save status checkpoint after import finished, and then clean up the engine.

tidb/br/pkg/lightning/restore/table_restore.go

Lines 853 to 858 in 550c7d0

	err := closedEngine.Import(ctx)
	rc.saveStatusCheckpoint(tr.tableName, engineID, err, checkpoints.CheckpointStatusImported)
	// Also cleanup engine when encountered ErrDuplicateDetected, since all duplicates kv pairs are recorded.
	if err == nil {
	err = multierr.Append(err, closedEngine.Cleanup(ctx))
	}

But saveStatusCheckpoint saves checkpoint asynchronously, an engine may be cleaned up successfully but exit before update checkpoint. After restart, verifyLocalFile will fail.

tidb/br/pkg/lightning/restore/restore.go

Line 827 in 550c7d0

func verifyLocalFile(ctx context.Context, cpdb checkpoints.DB, dir string) error {

To solve this issue, we can save status synchronously. An engine can only be cleaned up after the checkpoint has been successfully saved. Since we don't change the status of the engine very often, I think synchronously saving status checkpoint is acceptable and it can also avoid some unusual situations.

Related unstable integration test: lightning_checkpoint_engines_order.

What is changed and how it works?

Save save status checkpoint synchronously.

Check List

Tests

• Unit test
• Integration test

Release note

lightning: save status checkpoint synchronously

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• glorv
• 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]

/sig migrate
/cc @glorv @kennytm

[kennytm]

/component lightning
/run-check_dev_2 #26947

[glorv]

[2021-08-12T17:40:18.849Z] FAIL: restore_test.go:1001: tableRestoreSuite.TestSaveStatusCheckpoint

[2021-08-12T17:40:18.849Z] 

[2021-08-12T17:40:18.849Z] restore_test.go:1025:

[2021-08-12T17:40:18.849Z]     c.Assert(elapsed, GreaterEqual, time.Millisecond*100)

[2021-08-12T17:40:18.849Z] ... compare_one time.Duration = 77689 ("77.689µs")

[2021-08-12T17:40:18.849Z] ... compare_two time.Duration = 100000000 ("100ms")

@gozssky

[sleepymole]

[2021-08-12T17:40:18.849Z] FAIL: restore_test.go:1001: tableRestoreSuite.TestSaveStatusCheckpoint

[2021-08-12T17:40:18.849Z] 

[2021-08-12T17:40:18.849Z] restore_test.go:1025:

[2021-08-12T17:40:18.849Z]     c.Assert(elapsed, GreaterEqual, time.Millisecond*100)

[2021-08-12T17:40:18.849Z] ... compare_one time.Duration = 77689 ("77.689µs")

[2021-08-12T17:40:18.849Z] ... compare_two time.Duration = 100000000 ("100ms")

@gozssky

@glorv Fixed.

[kennytm]

rest LGTM

[glorv]

/run-check_dev_2

[sleepymole]

/run-integraion-tests

[kennytm]

/merge

[ti-chi-bot]

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

Commit hash: ff3ca70

[ti-chi-bot]

@gozssky: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

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 ti-community-infra/tichi repository.