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

backend: split and ingest region size more precise #369

Merged
merged 26 commits into from
Sep 3, 2020
Merged

Conversation

glorv
Copy link
Contributor

@glorv glorv commented Aug 12, 2020

What problem does this PR solve?

try to resolve #365 by avoid writing and ingesting big regions

What is changed and how it works?

make ingest region size not exceed regionSplitSize by:

  • use sample data to calculate kv range for each region.
  • Sample. When split range, first split the range from start-key to end-key to (total kv count) / 100 ranges. Then seek to the key that is greater or equal to each range start. If the seeked key is smaller than the range end, add the range end to sample values. If there is enough sampled values, then split the sample values to the target count and each split end with be the range end. If there are no enough sample values, we will fall back to the old naive way to split, but will use the db iterator the check empty ranges and skip them.
  • Region Size. when split regions, use 3/4 of the splitRegionSize to calculate the split size. And when ingest, use 5/4 splitRegionSize as the threshold. At each write and ingest phase, write no more than this threshold byte to tikv and ingest it. If there are remian kvs, split target region and try to ingest the remain bytes. By this way, we can make sure each region size will not exceed 5/4 * splitRegionSize in bytes.

This pr also emit some useless logs to make the error and warn logs less annoying.

Check List

Tests

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

Side effects

  • Possible performance regression
    • benchmark with 14k warehouse tpcc data shows no performance regression
  • Increased code complexity

Related changes

@glorv glorv requested a review from kennytm August 12, 2020 11:01
@glorv glorv changed the title backend: make write and ingest region more precise backend: split and ingest region size more precise Aug 12, 2020
@glorv
Copy link
Contributor Author

glorv commented Aug 12, 2020

sad for the code coverage decrease, the new logic for split need large scale test data

@breezewish
Copy link
Member

breezewish commented Aug 13, 2020

I'm not sure what's the implementation in this PR, but a kind reminder: In TiKV there are two region split methods, by size and by number of keys. When size or number of keys exceed the threshold the region will be splited. In lightning we'd better respect both mechanism, to avoid too many keys (which will also slow down the scanning).

@glorv
Copy link
Contributor Author

glorv commented Aug 14, 2020

I'm not sure what's the implementation in this PR, but a kind reminder: In TiKV there are two region split methods, by size and by number of keys. When size or number of keys exceed the threshold the region will be splited. In lightning we'd better respect both mechanism, to avoid too many keys (which will also slow down the scanning).

Thanks for the reminder. I update the split regions by take both size in bytes and key-value count into account.

lightning/backend/local.go Outdated Show resolved Hide resolved
lightning/backend/local.go Outdated Show resolved Hide resolved
@glorv
Copy link
Contributor Author

glorv commented Aug 31, 2020

/run-all-tests

@glorv glorv requested a review from 3pointer September 1, 2020 03:30
@glorv
Copy link
Contributor Author

glorv commented Sep 1, 2020

@kennytm @3pointer @YuJuncen PTAL again

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.

Rest LGTM

lightning/backend/localhelper.go Show resolved Hide resolved
lightning/backend/local.go Outdated Show resolved Hide resolved
lightning/backend/local.go 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

@kennytm kennytm added the status/LGT1 One reviewer already commented LGTM (LGTM1) label Sep 3, 2020
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/LGT1 One reviewer already commented LGTM (LGTM1) labels Sep 3, 2020
@glorv glorv merged commit 3cee9d5 into master Sep 3, 2020
@glorv glorv deleted the opt-local-ingest branch September 3, 2020 08:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

checksum fails easily by tikv timeout when restore big table with local backend
4 participants