You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "wanghenshui (via GitHub)" <gi...@apache.org> on 2023/05/06 07:41:26 UTC
[GitHub] [incubator-kvrocks] wanghenshui opened a new pull request, #1424: feat: rocksdb support auto tune with default config
wanghenshui opened a new pull request, #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424
(no comment)
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1542310412
@xiaobiaozhao Did you test 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] xiaobiaozhao commented on pull request #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "xiaobiaozhao (via GitHub)" <gi...@apache.org>.
xiaobiaozhao commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1537153049
```
# The maximum allowed aggregated write rate of flush and compaction (in MB/s).
# If the rate exceeds max-io-mb, io will slow down.
# 0 is no limit
# Default: 500
max-io-mb 500
```
I think you can reuse this configuration 'max-io-mb'. If it is 0 use auto-tune.
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "xiaobiaozhao (via GitHub)" <gi...@apache.org>.
xiaobiaozhao commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1546521178
Press data into kvrocks script
> docker run -d --rm redislabs/memtier_benchmark:latest -t 4 -c 20 -s 172.27.255.244 -p 6666 --distinct-client-seed --key-minimum=1000000000 --key-maximum=10000000000 --random-data --key-prefix="" --data-size=50 -n 100000000 --pipeline=100 --command "mset __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__ __key__ __data__"
write script
> docker run --rm redislabs/memtier_benchmark:latest -t 4 -c 20 -s 172.27.255.244 -p 6666 --distinct-client-seed --key-minimum=1000000000 --key-maximum=10000000000 --key-prefix="" -n 1000 --command "set __key__ __data__"
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "xiaobiaozhao (via GitHub)" <gi...@apache.org>.
xiaobiaozhao commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1545007233
>
> @xiaobiaozhao Did you test this PR?
I will test it this weekend.
--
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 #1424: feat: rocksdb support auto tune with default config
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#discussion_r1186659563
##########
src/storage/storage.cc:
##########
@@ -159,8 +159,19 @@ rocksdb::Options Storage::InitRocksDBOptions() {
options.sst_file_manager = sst_file_manager_;
int64_t max_io_mb = kIORateLimitMaxMb;
if (config_->max_io_mb > 0) max_io_mb = config_->max_io_mb;
- rate_limiter_ =
+ if (config_->rocks_db.rate_limiter_auto_tuned) {
+ rate_limiter_ =
+ std::shared_ptr<rocksdb::RateLimiter>(rocksdb::NewGenericRateLimiter(
+ max_io_mb * static_cast<int64_t>(MiB),
+ 100 * 1000, /* default */
+ 10, /* default */
+ rocksdb::RateLimiter::Mode::kWritesOnly,
+ true
+ ));
+ } else {
+ rate_limiter_ =
std::shared_ptr<rocksdb::RateLimiter>(rocksdb::NewGenericRateLimiter(max_io_mb * static_cast<int64_t>(MiB)));
+ }
Review Comment:
```suggestion
rate_limiter_ =
std::shared_ptr<rocksdb::RateLimiter>(rocksdb::NewGenericRateLimiter(
max_io_mb * static_cast<int64_t>(MiB),
100 * 1000, /* default */
10, /* default */
rocksdb::RateLimiter::Mode::kWritesOnly,
config_->rocks_db.rate_limiter_auto_tuned
));
```
--
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] mapleFU commented on pull request #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1546528287
Thanks! Do you have idea that why the performance decline in normal p100?
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk merged PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424
--
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 pull request #1424: feat: rocksdb support auto tune with default config
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1537082040
To make the CI pass, you can run `./x.py format` in your local before push your code.
--
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] wanghenshui commented on pull request #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "wanghenshui (via GitHub)" <gi...@apache.org>.
wanghenshui commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1537153479
> ```
> # The maximum allowed aggregated write rate of flush and compaction (in MB/s).
> # If the rate exceeds max-io-mb, io will slow down.
> # 0 is no limit
> # Default: 500
> max-io-mb 500
> ```
>
> I think you can reuse this configuration 'max-io-mb'. If it is 0 use auto-tune.
`auto tune` donesn't means auto detect write bandwidth, write bandwidth limit is always useful
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "xiaobiaozhao (via GitHub)" <gi...@apache.org>.
xiaobiaozhao commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1546811037
> Thanks! Do you have idea that why the performance decline in normal p100(And does all performance optimization and decline comes from unstable test result)?
I think this is bias of a test, p99 p9999 should be more convincing
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1550663195
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] mapleFU commented on a diff in pull request #1424: feat: rocksdb support auto tune with default config
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on code in PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#discussion_r1186660121
##########
src/storage/storage.cc:
##########
@@ -159,8 +159,19 @@ rocksdb::Options Storage::InitRocksDBOptions() {
options.sst_file_manager = sst_file_manager_;
int64_t max_io_mb = kIORateLimitMaxMb;
if (config_->max_io_mb > 0) max_io_mb = config_->max_io_mb;
- rate_limiter_ =
Review Comment:
What about:
```c++
rate_limiter_ =
std::shared_ptr<rocksdb::RateLimiter>(rocksdb::NewGenericRateLimiter(
max_io_mb * static_cast<int64_t>(MiB),
100 * 1000, /* default */
10, /* default */
rocksdb::RateLimiter::Mode::kWritesOnly,
config_->rocks_db.rate_limiter_auto_tuned
));
```
--
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] wanghenshui commented on a diff in pull request #1424: feat: rocksdb support auto tune with default config
Posted by "wanghenshui (via GitHub)" <gi...@apache.org>.
wanghenshui commented on code in PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#discussion_r1186662443
##########
src/storage/storage.cc:
##########
@@ -159,8 +159,19 @@ rocksdb::Options Storage::InitRocksDBOptions() {
options.sst_file_manager = sst_file_manager_;
int64_t max_io_mb = kIORateLimitMaxMb;
if (config_->max_io_mb > 0) max_io_mb = config_->max_io_mb;
- rate_limiter_ =
Review Comment:
sure
--
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] wanghenshui commented on pull request #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "wanghenshui (via GitHub)" <gi...@apache.org>.
wanghenshui commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1537093798
@xiaobiaozhao please benchmark this in `compaction with heavy write` scenario
--
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 #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1547099176
This option may bring better performance benefits when the background compaction is heavy, not sure if the benchmark data was under this scenario. But anyway, it's good to merge since it also brings slight performance improvement.
--
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] mapleFU commented on pull request #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#issuecomment-1537100514
This LGTM. Let wait some benchmarking for 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] PragmaTwice commented on a diff in pull request #1424: Support RocksDB auto-tune rate limiter for the background IO
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1424:
URL: https://github.com/apache/incubator-kvrocks/pull/1424#discussion_r1186706180
##########
tests/cppunit/config_test.cc:
##########
@@ -79,6 +79,7 @@ TEST(Config, GetAndSet) {
{"rocksdb.max_bytes_for_level_multiplier", "10"},
{"rocksdb.level_compaction_dynamic_level_bytes", "yes"},
{"rocksdb.max_background_jobs", "4"},
+
Review Comment:
```suggestion
```
We do not expect newline without context : )
--
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