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

lightning/restore: support ingset multi ssts for same range #1089

Merged
merged 13 commits into from
May 27, 2021

Conversation

glorv
Copy link
Collaborator

@glorv glorv commented May 7, 2021

What problem does this PR solve?

Ingest multi SST files (different CF) together to gain better consistency. This can resolve the problem that lighting import or BR restore may cause TiFlash panic.

What is changed and how it works?

Check List

Tests

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

Code changes

  • Has exported function/method change
  • Has interface methods change

Side effects

Related changes

  • Need to cherry-pick to the release branch

Release Note

  • No Release Note

@glorv glorv force-pushed the ingest-multi-cf branch from 449398d to dbcd2c3 Compare May 7, 2021 12:00
@glorv glorv force-pushed the ingest-multi-cf branch from aa23c57 to 6c2d6b2 Compare May 13, 2021 08:36
@glorv
Copy link
Collaborator Author

glorv commented May 13, 2021

/build

@glorv glorv marked this pull request as ready for review May 13, 2021 12:45
Copy link
Collaborator

@Little-Wallace Little-Wallace left a comment

Choose a reason for hiding this comment

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

LGTM

if err != nil {
if s, ok := status.FromError(err); ok {
if s.Code() == codes.Unimplemented {
local.supportMultiIngest = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Does tiflash need to implement such interfaces? @Little-Wallace

Copy link
Collaborator

Choose a reason for hiding this comment

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

No

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that supportMultiIngest is true only when all stores have implemented related interfaces.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MultiIngest is only called by region leader, so tiflash needn't implement this interface. We should skip check tiflash here

idx++
}

return files[idx:], files[idx:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

files[:idx], files[idx:] 🤔️?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

😂

@kennytm
Copy link
Collaborator

kennytm commented May 27, 2021

/lgtm

@ti-chi-bot
Copy link
Member

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • Little-Wallace
  • 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 writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

@ti-chi-bot ti-chi-bot added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels May 27, 2021
@glorv glorv merged commit 179e15d into pingcap:master May 27, 2021
@glorv glorv deleted the ingest-multi-cf branch May 27, 2021 05:08
ti-chi-bot pushed a commit to ti-chi-bot/br that referenced this pull request May 27, 2021
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1149.

@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request could not be created: failed to create pull request against pingcap/br#release-4.0 from head ti-chi-bot:cherry-pick-1089-to-release-4.0: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-1089-to-release-4.0."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

ti-chi-bot pushed a commit to ti-chi-bot/br that referenced this pull request May 27, 2021
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot pushed a commit to ti-chi-bot/br that referenced this pull request May 27, 2021
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created: #1150.

ti-chi-bot pushed a commit to ti-chi-bot/br that referenced this pull request May 27, 2021
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request could not be created: failed to create pull request against pingcap/br#release-5.0 from head ti-chi-bot:cherry-pick-1089-to-release-5.0: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for ti-chi-bot:cherry-pick-1089-to-release-5.0."}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

3pointer added a commit to 3pointer/br that referenced this pull request Jun 4, 2021
backup/checksum: add cluster index support for ranges (pingcap#1120)

lightning/restore: support ingset multi ssts for same range (pingcap#1089)

* ingest ssts for the same range in a batch

* make ingest compatible with old tikv

tests: add row count check for br clustered index tests (pingcap#1151)

storage: mkdirAll for local storage even when SkipCheckPath is true (pingcap#1156)

tests/br: fix bug in issue pingcap#1158 (pingcap#1160)

tests/br/compatibility: fix failed for manifest unknown (pingcap#1161)

*: upgrade go version from 1.13/1.15 to 1.16 (pingcap#1159)

action trigger: fix compatibility trigger bug on push (pingcap#1170)

address comment

fix test

address comment

address comment

fix build

add ut for ignore columns

go.mod: update tidb to the new version (pingcap#1153)

lightning: support restore data into tables that contains data (pingcap#784)

fix conflict after merge master
3pointer pushed a commit to 3pointer/br that referenced this pull request Jun 4, 2021
…1089)

* ingest ssts for the same range in a batch

* make ingest compatible with old tikv

* fix lint

* fix meta length check

* update tidb

* do not check tiflash

* add log

* fix

* fix

* fix

* fix
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.

6 participants