Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

pkg/lightning: replace local generated ts with tso fetch from pd #850

Merged
merged 3 commits into from
Mar 12, 2021

Conversation

glorv
Copy link
Collaborator

@glorv glorv commented Mar 11, 2021

What problem does this PR solve?

Replace commit ts generated with local time by tso fetch from pd. This can address the issue that if local time is mush faster than tidb cluster time, after import data with local/importer backend, query may returns empty result.

What is changed and how it works?

  • Fetch a ts from pd and store it in RestoreController and use this ts for all WriteRows
  • Always fetch the newest ts from pd when do table checksum. This is useful when we support concurrent import with many lightnings and only do checksum after the last lightning finished.

Check List

Tests

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

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • Fix the bug that lightning generated ts may be to large or small that query may return incorrect result.

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM for now.

i don't particularly like requiring TS for TiDB backend, but let's refactor this later.

pkg/lightning/restore/restore.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@ti-srebot ti-srebot added the status/LGT1 LGTM1 label Mar 11, 2021
@glorv
Copy link
Collaborator Author

glorv commented Mar 12, 2021

@lance6716 @Little-Wallace PTAL

@lance6716
Copy link
Contributor

/lgtm

@ti-srebot ti-srebot removed the status/LGT1 LGTM1 label Mar 12, 2021
@ti-srebot ti-srebot added the status/LGT2 LGTM2 label Mar 12, 2021
@glorv
Copy link
Collaborator Author

glorv commented Mar 12, 2021

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 8a3f5e2 into pingcap:master Mar 12, 2021
ti-srebot pushed a commit to ti-srebot/br that referenced this pull request Mar 12, 2021
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #860

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants