You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2023/01/02 14:43:19 UTC
[GitHub] [incubator-kvrocks] xiaobiaozhao opened a new pull request, #1215: perf: turn on aync_io
xiaobiaozhao opened a new pull request, #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215
Here are some comparisons between `async_io` and `normal`
## big value 500B
<img width="955" alt="image" src="https://user-images.githubusercontent.com/52393536/210245022-a8b1bdbf-090b-4374-8bc2-f03871e4b887.png">
## small value 50B
<img width="952" alt="image" src="https://user-images.githubusercontent.com/52393536/210245253-badbc0e1-b953-41dc-b908-9fee138519a1.png">
## monitor diff
The green line is kvrocks compiled by async ioļ¼this pr.
`ts*` command using seek function to scan data from db.
## summarize
| | cpu | mem | latency | qps | |
| ------ | ---- | ---- | -------- | ---- | --- |
| async | 110% | 100% | 100% | 120% | |
| normal | 100% | 100% | 130-400% | 100% | |
`async_io` is better in seek read, higher qps, lower latency, and slightly higher cpu usage. Perfect for introducing projects
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1082075068
##########
src/storage/storage.cc:
##########
@@ -103,6 +103,11 @@ void Storage::SetWriteOptions(const Config::RocksDB::WriteOptions &config) {
write_opts_.memtable_insert_hint_per_batch = config.memtable_insert_hint_per_batch;
}
+void Storage::SetReadOptions(rocksdb::ReadOptions &read_options) {
Review Comment:
Yes, maybe `SetSeekOptions` is more precise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1089605098
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
Sure, it looks more clear to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
torwig commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1068159861
##########
src/config/config.cc:
##########
@@ -204,6 +204,9 @@ Config::Config() {
{"rocksdb.write_options.low_pri", true, new YesNoField(&RocksDB.write_options.low_pri, false)},
{"rocksdb.write_options.memtable_insert_hint_per_batch", true,
new YesNoField(&RocksDB.write_options.memtable_insert_hint_per_batch, false)},
+
+ /* rocksdb read options */
+ {"rocksdb.read_options.async_io", false, new YesNoField(&RocksDB.read_options.async_io, true)},
Review Comment:
Should it be YesNoField(&RocksDB.read_options.async_io, **false**) here?
##########
src/storage/storage.cc:
##########
@@ -103,6 +103,11 @@ void Storage::SetWriteOptions(const Config::RocksDB::WriteOptions &config) {
write_opts_.memtable_insert_hint_per_batch = config.memtable_insert_hint_per_batch;
}
+void Storage::SetReadOptions(rocksdb::ReadOptions &read_options) {
Review Comment:
It sounds like the `SetReadOptions` function sets the specified `read_options` to the whole `Storage` but the function doesn't do that. For me, the prior name of the function wasn't ideal but definitely more clear :)
The code you are trying to eliminate is repetitive but it's tricky to do that.
I recommend passing `read_options` by a pointer to stress that this parameter will be changed and can be considered as output.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1401811925
@xiaobiaozhao Good research.
If you update your branch, and make a final change as mentioned @git-hulk, I think we can merge this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1215: perf: turn on aync_io
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1369019805
Cool, I think it'd be better if we have the option to let users determine whether to enable it or not.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1089604572
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
@PragmaTwice Does this comment look better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1407281279
Thanks all, merging...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
xiaobiaozhao commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1068192344
##########
src/storage/storage.cc:
##########
@@ -103,6 +103,11 @@ void Storage::SetWriteOptions(const Config::RocksDB::WriteOptions &config) {
write_opts_.memtable_insert_hint_per_batch = config.memtable_insert_hint_per_batch;
}
+void Storage::SetReadOptions(rocksdb::ReadOptions &read_options) {
Review Comment:
According to the official document, mutil_get will also improve the performance. I think we can add this flag to all readoptions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1397874201
> Good feature, any updates? In latest rocksdb update fixed some bugs in async_io support, so this PR is very good point to improve our performance.
@xiaobiaozhao has updated the PR, to see if @torwig and @PragmaTwice have comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "xiaobiaozhao (via GitHub)" <gi...@apache.org>.
xiaobiaozhao commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1401806271
I've tested mget and hgetall and async_io obviously gives hgetall a big boost.
If rocksdb has better updates in the future, we can continue to follow up.
<img width="1063" alt="image" src="https://user-images.githubusercontent.com/52393536/214282941-dd5cd92a-e5fb-4522-8736-b07e3b1e6297.png">
<img width="1068" alt="image" src="https://user-images.githubusercontent.com/52393536/214282895-e6d0300a-5efa-477b-9429-f7c8baa135a8.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1414695218
Hi @jishengming1 I think we can move to the discussion to make the PR context clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] jishengming1 commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "jishengming1 (via GitHub)" <gi...@apache.org>.
jishengming1 commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1414686177
>
Excuse me again, one more question.
In the same link, if request 1 blocks, will all subsequent requests block? Is request2 affected by request1?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1377365147
@xiaobiaozhao It's not right to involve the server in the storage layer. You should add the async_io flag in storage or implement like the write option https://github.com/apache/incubator-kvrocks/blob/unstable/src/storage/storage.cc#L76
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
xiaobiaozhao commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1068189269
##########
src/config/config.cc:
##########
@@ -204,6 +204,9 @@ Config::Config() {
{"rocksdb.write_options.low_pri", true, new YesNoField(&RocksDB.write_options.low_pri, false)},
{"rocksdb.write_options.memtable_insert_hint_per_batch", true,
new YesNoField(&RocksDB.write_options.memtable_insert_hint_per_batch, false)},
+
+ /* rocksdb read options */
+ {"rocksdb.read_options.async_io", false, new YesNoField(&RocksDB.read_options.async_io, true)},
Review Comment:
yes, I missed this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
xiaobiaozhao commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1066994961
##########
kvrocks.conf:
##########
@@ -721,6 +721,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
+
+# Deafult yes
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] jishengming1 commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "jishengming1 (via GitHub)" <gi...@apache.org>.
jishengming1 commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1414858756
> context
Okay, I'll go to the discussion forum to discuss this issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] jishengming1 commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "jishengming1 (via GitHub)" <gi...@apache.org>.
jishengming1 commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1413422424
> Yes, exactly. The order of the response is always exactly the same as the request.
Thank you very much.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] xiaobiaozhao commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
xiaobiaozhao commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1066151074
##########
kvrocks.conf:
##########
@@ -721,6 +721,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
+
+# Deafult yes
+rocksdb.read_options.async_io yes
Review Comment:
@git-hulk
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
torwig commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1064835498
##########
kvrocks.conf:
##########
@@ -721,6 +721,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
+
+# Deafult yes
+rocksdb.read_options.async_io yes
Review Comment:
Perhaps, it would be better to set the default value to `false` because this feature is experimental in RocksDB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1085387479
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
I cannot exactly understand the meaning of this comment, could you improve it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] jishengming1 commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "jishengming1 (via GitHub)" <gi...@apache.org>.
jishengming1 commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1413350402
Hi @xiaobiaozhao , Excuse me again, one more question.
Is the response and request order consistent at the same link? For example, send request 1, request 2 and expect to receive response 1, response 2 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1089604487
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
```suggestion
# This feature only takes effect in Iterators and MultiGet.
# If yes, RocksDB will try to read asynchronously and in parallel as much as possible to hide IO latency.
# In iterators, it will prefetch data asynchronously in the background for each file being iterated on.
# In MultiGet, it will read the necessary data blocks from those files in parallel as much as possible.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1066458969
##########
kvrocks.conf:
##########
@@ -721,6 +721,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
+
+# Deafult yes
Review Comment:
typo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
torwig commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1068202018
##########
src/storage/storage.cc:
##########
@@ -103,6 +103,11 @@ void Storage::SetWriteOptions(const Config::RocksDB::WriteOptions &config) {
write_opts_.memtable_insert_hint_per_batch = config.memtable_insert_hint_per_batch;
}
+void Storage::SetReadOptions(rocksdb::ReadOptions &read_options) {
Review Comment:
I meant that the `Storage` class stores the `write_options` variable as its member, but the `ReadOptions` is passed to the iterator's constructor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] aleksraiden commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
aleksraiden commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1396979977
Good feature, any updates? In latest rocksdb update fixed some bugs in async_io support, so this PR is very good point to improve our performance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk merged pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk merged PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1067121966
##########
kvrocks.conf:
##########
@@ -721,6 +721,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
+
+# Deafult yes
+rocksdb.read_options.async_io yes
Review Comment:
Agree with @torwig, can turn it to false by default and wait for this feature production ready. But we can encourage users to have a try.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1068059260
##########
src/storage/storage.h:
##########
@@ -68,6 +68,7 @@ class Storage {
~Storage();
void SetWriteOptions(const Config::RocksDB::WriteOptions &config);
+ void FillSeekReadOptions(rocksdb::ReadOptions &read_options);
Review Comment:
I think we can rename `FillSeekReadOptions` => `SetReadOptions ` to keep consistent with `SetWriteOptions`.
##########
src/config/config.cc:
##########
@@ -204,6 +204,8 @@ Config::Config() {
{"rocksdb.write_options.low_pri", true, new YesNoField(&RocksDB.write_options.low_pri, false)},
{"rocksdb.write_options.memtable_insert_hint_per_batch", true,
new YesNoField(&RocksDB.write_options.memtable_insert_hint_per_batch, false)},
+ /* rocksdb read options */
+ {"rocksdb.read_options.async_io", false, new YesNoField(&RocksDB.read_options.async_io, true)},
Review Comment:
```suggestion
{"rocksdb.read_options.async_io", false, new YesNoField(&RocksDB.read_options.async_io, true)},
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1085387479
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
> RocksDB apply it if reads are sequential and its internal automatic prefetching.
I cannot exactly understand the meaning of this comment, could you improve it?
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
> RocksDB apply it if reads are sequential and its internal automatic prefetching.
I cannot exactly understand the meaning of this comment, could you improve it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#discussion_r1089604487
##########
kvrocks.conf:
##########
@@ -726,6 +726,13 @@ rocksdb.max_bytes_for_level_base 268435456
# Default: 10
rocksdb.max_bytes_for_level_multiplier 10
+# If yes, RocksDB will prefetch some of data asynchronously.
+# RocksDB apply it if reads are sequential and its internal automatic
+# prefetching.
Review Comment:
```suggestion
# If yes, RocksDB will try to read asynchronously and in parallel as much as possible to hide IO latency.
# This feature only takes effect in Iterators and MultiGet.
# In iterators, it will prefetch data asynchronously in the background for each file being iterated on.
# In MultiGet, it will read the necessary data blocks from those files in parallel as much as possible.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1215: Allow to enable the async_io option to improve the performance
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1413391065
@jishengming1 Yes, exactly. The order of the response is always exactly the same as the request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kvrocks] torwig commented on pull request #1215: perf: turn on aync_io
Posted by GitBox <gi...@apache.org>.
torwig commented on PR #1215:
URL: https://github.com/apache/incubator-kvrocks/pull/1215#issuecomment-1369031250
Since this option is marked as `Experimental` in the `rocksdb` codebase, I agree to make this option configurable.
@xiaobiaozhao Thank you for the benchmarks. Nice job!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org