You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "mapleFU (via GitHub)" <gi...@apache.org> on 2023/07/10 06:01:50 UTC

[GitHub] [kvrocks] mapleFU opened a new issue, #1572: Style: Change `Storage::SetReadOptions` to `Storage::DefaultScanOptions`

mapleFU opened a new issue, #1572:
URL: https://github.com/apache/kvrocks/issues/1572

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/kvrocks/issues) and found no similar issues.
   
   
   ### Motivation
   
   ```c++
   class Storage {
    public:
     explicit Storage(Config *config);
     
     void SetWriteOptions(const Config::RocksDB::WriteOptions &config);
     void SetReadOptions(rocksdb::ReadOptions &read_options);
   
     const rocksdb::WriteOptions &DefaultWriteOptions() { return write_opts_; }
   };
   ```
   
   Now kvrocks has `SetWriteOptions` and `SetReadOptions`. However, there syntax doesn't same. `SetWriteOptions` would set the `Storage` internal `rocksdb::WriteOptions` during intialize, and the detail option can be get by `DefaultWriteOptions()`.
   
   However, `SetReadOptions` only does some fixup when called:
   
   ```c++
   void Storage::SetReadOptions(rocksdb::ReadOptions &read_options) {
     read_options.fill_cache = false;
     read_options.async_io = config_->rocks_db.read_options.async_io;
   }
   ```
   
   Would we create a `DefaultScanOptions`, and remove `SetReadOptions` for that ?
   
   ### Solution
   
   ```c++
   class Storage {
    public:
     explicit Storage(Config *config);
     
     void SetWriteOptions(const Config::RocksDB::WriteOptions &config);
   -  void SetReadOptions(rocksdb::ReadOptions &read_options);
   
     const rocksdb::WriteOptions &DefaultWriteOptions() { return write_opts_; }
   + rocksdb::ReadOptions DefaultScanOptions();
   };
   ```
   
   ### Are you willing to submit a PR?
   
   - [X] I'm willing to submit a 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.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kvrocks] git-hulk closed issue #1572: Style: Change `Storage::SetReadOptions` to `Storage::DefaultScanOptions`

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk closed issue #1572: Style: Change `Storage::SetReadOptions` to `Storage::DefaultScanOptions`
URL: https://github.com/apache/kvrocks/issues/1572


-- 
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] [kvrocks] git-hulk commented on issue #1572: Style: Change `Storage::SetReadOptions` to `Storage::DefaultScanOptions`

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on issue #1572:
URL: https://github.com/apache/kvrocks/issues/1572#issuecomment-1628403873

   Sounds good 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