Skip to content
This repository was archived by the owner on Dec 8, 2021. It is now read-only.

backend/local: batch split region with batch limit #487

Merged
merged 23 commits into from
Nov 27, 2020
Merged

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Nov 24, 2020

What problem does this PR solve?

Limit the batch split key size when batch split regions

close #481

Need pingcap/br#612 to be merged first.

What is changed and how it works?

  • Run batch split in a loop to limit each request's key size
  • Add retry for pagnateScanRegion and scatter region
  • Add some log

Check List

Tests

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

Side effects

Related changes

@glorv glorv requested review from kennytm and 3pointer November 24, 2020 07:00
@kennytm kennytm added the status/DNM Do not merge, test is failing or blocked by another PR label Nov 24, 2020
@glorv glorv added this to the 4.0.9 milestone Nov 25, 2020
@glorv glorv added status/PTAL This PR is ready for review. Add this label back after committing new changes and removed status/DNM Do not merge, test is failing or blocked by another PR labels Nov 26, 2020
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Could you add a unit test?

Comment on lines 12 to 13
"github.com/pingcap/br/pkg/utils"

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"github.com/pingcap/br/pkg/utils"
"github.com/pingcap/br/pkg/utils"

Little-Wallace
Little-Wallace previously approved these changes Nov 27, 2020
Copy link
Contributor

@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

Copy link
Contributor

@3pointer 3pointer left a comment

Choose a reason for hiding this comment

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

LGTM

@glorv glorv added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Nov 27, 2020
@glorv glorv added status/LGT1 One reviewer already commented LGTM (LGTM1) and removed status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) labels Nov 27, 2020
@overvenus overvenus merged commit 8a276c0 into master Nov 27, 2020
@glorv glorv deleted the batch-split branch December 16, 2020 02:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/LGT1 One reviewer already commented LGTM (LGTM1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lightning needs to set an upper limit of split keys in one BatchSplit RPC call
5 participants