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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
