Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support batching operations together #11

Closed
wants to merge 2 commits into from
Closed

Conversation

mvayngrib
Copy link
Contributor

@mvayngrib mvayngrib commented Jul 1, 2020

the problem

set/delete are currently per-key, and result in the entire file being written

solution

a batch api lets the caller maintain a kind of queue, and flush all writes at the end

const promise = store.batch()
  // add as many operations as you want
  .add({ type: 'set', key: 'abc', value: '123' })
  .add({ type: 'delete', key: 'efg' })
  // flush
  .exec()

the main problem will probably be solved by https://github.com/ExodusMovement/exodus-desktop/pull/8034 and https://github.com/ExodusMovement/exodus-desktop/pull/8035 but it would still be nice to be able to batch ops together and save time on writes

the below is what i get on restore on master with batching added on top of master (ignore the times, the blue numbers are the batch sizes):
batcher

when running on top of the PRs linked above, it's way better, but might still be worth it in case we have bursts:
image

@mvayngrib mvayngrib self-assigned this Jul 1, 2020
@mvayngrib mvayngrib requested a review from RyanZim July 1, 2020 23:32
@kklash
Copy link
Contributor

kklash commented Jul 1, 2020

Unclear on context of the problem that this PR solves. Please elaborate PR description with some background.

@mvayngrib
Copy link
Contributor Author

@kklash haha ur absolutely right. Does the updated description clarify things?

@mvayngrib mvayngrib requested a review from kklash July 1, 2020 23:41
@kklash
Copy link
Contributor

kklash commented Jul 1, 2020

Yes, thanks @mvayngrib

@mvayngrib
Copy link
Contributor Author

closing in favor of one or more of #12 #13 #14

@mvayngrib mvayngrib closed this Jul 3, 2020
@kklash kklash deleted the mv/batch branch July 3, 2020 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants