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.

Sign up for GitHub

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

Jump to bottom

Storage: Extract an MultiGet Options for rocksdb::MultiGet #1582

Merged
merged 3 commits into from
Jul 13, 2023
Merged

Storage: Extract an MultiGet Options for rocksdb::MultiGet #1582

merged 3 commits into from
Jul 13, 2023

Conversation

mapleFU
Copy link
Member

@mapleFU mapleFU commented Jul 12, 2023

We have two cases using rocksdb::MultiGet:

  1. String::getRawValues
  2. HSet::MGet

They share the different multiget configs, this patch unify them with:

  1. fill_cache = default
  2. async_io = config_->async ( note that MultiGet can benefits from async io)

@mapleFU mapleFU marked this pull request as draft July 12, 2023 06:29
@mapleFU
Copy link
Member Author

mapleFU commented Jul 12, 2023

I'm a little busy today. will visit this tonight

@mapleFU mapleFU force-pushed the storage/extract-mget-options branch from b242991 to 273825e Compare July 12, 2023 16:32
@mapleFU mapleFU marked this pull request as ready for review July 12, 2023 16:32
@mapleFU mapleFU force-pushed the storage/extract-mget-options branch from 273825e to b187333 Compare July 12, 2023 16:54
@xiaobiaozhao
Copy link
Contributor

Have you tested how much performance can be improved?

@mapleFU
Copy link
Member Author

mapleFU commented Jul 12, 2023

Currently not, I guess this would make HGet benefits from cache and async_io, but didn't test these commands

@mapleFU
Copy link
Member Author

mapleFU commented Jul 13, 2023

@git-hulk Would you mind take a look?

@git-hulk
Copy link
Member

@git-hulk Would you mind take a look?

Looks good to me.

@xiaobiaozhao I think this PR is only for follow up #1215

@PragmaTwice PragmaTwice merged commit 7af5bb5 into apache:unstable Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PragmaTwice PragmaTwice PragmaTwice approved these changes

@git-hulk git-hulk git-hulk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mapleFU @xiaobiaozhao @git-hulk @PragmaTwice