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

[docdb] Make rocksdb_compact_flush_rate_limit_bytes_per_sec be shared across rocksdbs on a node #10438

Closed
bmatican opened this issue Oct 28, 2021 · 1 comment
Closed

[docdb] Make rocksdb_compact_flush_rate_limit_bytes_per_sec be shared across rocksdbs on a node #10438

bmatican opened this issue Oct 28, 2021 · 1 comment
Assignees
@arybochkin
Labels
area/docdb YugabyteDB core features

Comments

@bmatican
Copy link
Contributor

bmatican commented Oct 28, 2021 •
edited
Loading

Description

Currently our disk throttle flag is independent per rocksdb, which makes it not be super useful, as we have 2 rocksdbs per tablet and can have thousand of tablets per server.

We should change the way we initialize rocksdb, so we have a shared throttle component, allowing us to use rocksdb_compact_flush_rate_limit_bytes_per_sec at a server level.

Code pointer: src/yb/docdb/docdb_rocksdb_util.cc

    if (FLAGS_rocksdb_compact_flush_rate_limit_bytes_per_sec > 0) {
      options->rate_limiter.reset(
          rocksdb::NewGenericRateLimiter(FLAGS_rocksdb_compact_flush_rate_limit_bytes_per_sec));
    }

Good people references: @ttyusupov @mbautin
@bmatican bmatican added the area/docdb YugabyteDB core features label Oct 28, 2021
@bmatican bmatican self-assigned this Oct 28, 2021
@bmatican bmatican assigned arybochkin and unassigned bmatican Nov 2, 2021
@bmatican bmatican mentioned this issue Dec 10, 2021
Open
arybochkin added a commit that referenced this issue Jan 13, 2022
@arybochkin
[#10438] docdb: Make rocksdb_compact_flush_rate_limit_bytes_per_sec b…
75f3124 
…e shared across rocksdbs on a node

Summary:
Adding a new flag `rocksdb_compact_flush_rate_limit_sharing_mode` to allow `rocksdb::RateLimiter` instance be shared across all the tablets for the particular tserver (currently any RocksDB instance has its own `rocksdb::RateLimiter` object). The flag can be set to `tserver` or `none`; the default value of the flag is set to `none` to keep current rate limiting algorithm as default until additional perf tests are done (and if perf is fine the flag's default value might be changed to `tserver`). Also the implementation prevents the case when the flag is switched from `tserver` to `none` on a running tserver to avoid a scenario with mixed mode for rate limiting.

Additionally fixed `TsTabletManagerTest.TestTabletReportLimit` failure (happened due to heartbeat is not off immediately in `SetUp()`)

Test Plan:
ybd --cxx-test tserver_ts_tablet_manager-test
ybd --cxx-test docdb_docdb_rocksdb_util-test
ybd --cxx-test docdb_docdb-test
ybd --cxx-test docdb_doc_operation-test
ybd --cxx-test docdb_randomized_docdb-test
ybd --cxx-test tablet_tablet-metadata-test
ybd --cxx-test tablet_tablet-test
ybd --cxx-test tablet_tablet-split-test
ybd --cxx-test client_ql-stress-test --gtest_filter QLStressTest.FlushCompact
ybd --cxx-test integration-tests_ts_tablet_manager-itest
ybd --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.PostSplitCompactionDoesntBlockTabletCleanup
ybd --cxx-test integration-tests_master_failover-itest --gtest_filter MasterFailoverTest.TestLoadMoveCompletion
ybd --java-test org.yb.loadtester.TestFullMoveWithHeartBeatDelay
ybd --java-test org.yb.loadtester.TestMasterLeaderDecommission
ctest -R ^rocksdb

Reviewers: timur, mbautin, sergei

Reviewed By: sergei

Subscribers: ybase, bogdan

Differential Revision: https://phabricator.dev.yugabyte.com/D14443
arybochkin added a commit that referenced this issue Mar 3, 2022
@arybochkin
[#10438] docdb: Changing default values for rocksdb_compact_flush_rat…
d68c7dd 
…e_limit_ flags

Summary:
Changing default values for flags `rocksdb_compact_flush_rate_limit_bytes_per_sec=1GB` and
`rocksdb_compact_flush_rate_limit_sharing_mode=tserver` on base of result of test runs for
sample apps and heavy workloads.

Test Plan:
ybd --cxx-test tserver_ts_tablet_manager-test --gtest_filter TsTabletManagerTest.RateLimiterSharing
ybd --cxx-test docdb_docdb_rocksdb_util-test --gtest_filter DocDBRocksDBUtilTest.RocksDBRateLimiter
ybd --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.PostSplitCompactionDoesntBlockTabletCleanup
ybd --cxx-test client_ql-stress-test --gtest_filter QLStressTest.DynamicCompactionPriority

Reviewers: bogdan

Reviewed By: bogdan

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D15564
@arybochkin
Copy link
Contributor

Basing on test results default values have been changed for:

rocksdb_compact_flush_rate_limit_bytes_per_sec=1GB
rocksdb_compact_flush_rate_limit_sharing_mode=tserver

jayant07-yb pushed a commit to jayant07-yb/yugabyte-db that referenced this issue Mar 8, 2022
@arybochkin
[yugabyte#10438] docdb: Changing default values for rocksdb_compact_f…
b597741 
…lush_rate_limit_ flags

Summary:
Changing default values for flags `rocksdb_compact_flush_rate_limit_bytes_per_sec=1GB` and
`rocksdb_compact_flush_rate_limit_sharing_mode=tserver` on base of result of test runs for
sample apps and heavy workloads.

Test Plan:
ybd --cxx-test tserver_ts_tablet_manager-test --gtest_filter TsTabletManagerTest.RateLimiterSharing
ybd --cxx-test docdb_docdb_rocksdb_util-test --gtest_filter DocDBRocksDBUtilTest.RocksDBRateLimiter
ybd --cxx-test integration-tests_tablet-split-itest --gtest_filter TabletSplitITest.PostSplitCompactionDoesntBlockTabletCleanup
ybd --cxx-test client_ql-stress-test --gtest_filter QLStressTest.DynamicCompactionPriority

Reviewers: bogdan

Reviewed By: bogdan

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D15564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arybochkin arybochkin

Labels
area/docdb YugabyteDB core features
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bmatican @arybochkin