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

restore: give priority to small tables for importing #156

Merged
merged 4 commits into from
Apr 9, 2019

Conversation

lonng
Copy link
Contributor

@lonng lonng commented Mar 28, 2019

What problem does this PR solve?

Put the large table in the front of the slice which can avoid large table take a long time to import and block small table to release index worker.

What is changed and how it works?

Sorting tables before restore.

Check List

Tests

  • Unit test
  • Integration test

Side effects

N/A

Related changes

N/A

@lonng lonng changed the base branch from master to kennytm/parse-faster March 28, 2019 04:59
@kennytm kennytm added status/DNM Do not merge, test is failing or blocked by another PR priority/important type/enhancement Performance improvement or refactoring labels Mar 28, 2019
@kennytm
Copy link
Collaborator

kennytm commented Apr 3, 2019

Please rebase on the latest master (if you still want to proceed on this PR).

@lonng lonng changed the base branch from kennytm/parse-faster to master April 4, 2019 02:22
@lonng lonng changed the title restore: give priority to large tables for importing restore: give priority to small tables for importing Apr 4, 2019
@kennytm kennytm removed the status/DNM Do not merge, test is failing or blocked by another PR label Apr 7, 2019
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.

lightning/restore/restore.go Outdated Show resolved Hide resolved
lightning/restore/restore.go Outdated Show resolved Hide resolved
@lonng
Copy link
Contributor Author

lonng commented Apr 8, 2019

PTAL @GregoryIan @kennytm

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 Apr 8, 2019
@IANTHEREAL
Copy link
Collaborator

LGTM

@lonng lonng added status/LGT2 Two reviewers already commented LGTM, ready for merge (LGTM2) and removed status/LGT1 One reviewer already commented LGTM (LGTM1) labels Apr 9, 2019
@lonng lonng merged commit c14d62a into master Apr 9, 2019
@lonng lonng deleted the lonng/priority-sched branch April 9, 2019 03:03
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) type/enhancement Performance improvement or refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants