Skip to content

Commit

Permalink
Merge 'hints: use a soft disk space limit in hints commitlog' from Pi…
Browse files Browse the repository at this point in the history
…otr Dulikowski

A recent change to the commitlog (4082f57) caused its configurable size limit to
be strictly enforced - after reaching the limit, new segments wouldn't be
allocated until some of the previous segments are freed. This flow can work for
the regular commitlog, however the hints commitlog does not delete the segments
itself - instead, hints manager recreates its commitlog every 10 seconds, picks
up segments left by the previous instance and deletes each segment manually only
after all hints are sent out from a segment.

Because of the non-standard flow, it is possible that the hints commitlog fills
up and stops accepting more hints. Hints manager uses a relatively low limit for
each commitlog instance (128MB divided by shard count), so it's not hard to fill
it up. What's worse, hints manager tries to acquire file_update_mutex in
exclusive mode before re-creating the commitlog, while hints waiting to be
written acquire this lock in shared mode - which causes hints flushing to
completely deadlock and no more hints be admitted to the commitlog. The queue of
hints waiting to be admitted grows very quickly and soon all writes which could
result in a hint being generated are rejected with OverloadedException.

To solve this problem, it is now possible to bring back the soft disk space
limit by setting a flag in commitlog's configuration.

Tests:
- unit(dev)
- wrote hints for 15 minutes in order to see if it gets stuck again

Fixes #8137

Closes #8206

* github.com:scylladb/scylla:
  hints_manager: don't use commitlog hard space limit
  commitlog: add an option to allow going over size limit
  • Loading branch information
psarna committed Mar 4, 2021
2 parents d6a94a7 + 376da49 commit added53
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 2 deletions.
4 changes: 2 additions & 2 deletions db/commitlog/commitlog.cc
Expand Up @@ -1499,7 +1499,7 @@ future<db::commitlog::segment_manager::sseg_ptr> db::commitlog::segment_manager:
});
}

if (max_disk_size != 0 && totals.total_size_on_disk >= max_disk_size) {
if (!cfg.allow_going_over_size_limit && max_disk_size != 0 && totals.total_size_on_disk >= max_disk_size) {
clogger.debug("Disk usage ({} MB) exceeds maximum ({} MB) - allocation will wait...", totals.total_size_on_disk/(1024*1024), max_disk_size/(1024*1024));
auto f = cfg.reuse_segments ? _recycled_segments.not_empty() : _disk_deletions.get_shared_future();
return f.then([this] {
Expand Down Expand Up @@ -1746,7 +1746,7 @@ future<> db::commitlog::segment_manager::delete_segments(std::vector<sstring> fi
return f.finally([&] {
// We allow reuse of the segment if the current disk size is less than shard max.
auto usage = totals.total_size_on_disk;
if (!_shutdown && cfg.reuse_segments) {
if (!_shutdown && cfg.reuse_segments && usage <= max_disk_size) {
descriptor d(next_id(), "Recycled-" + cfg.fname_prefix);
auto dst = this->filename(d);

Expand Down
1 change: 1 addition & 0 deletions db/commitlog/commitlog.hh
Expand Up @@ -140,6 +140,7 @@ public:
bool reuse_segments = true;
bool use_o_dsync = false;
bool warn_about_segments_left_on_disk_after_shutdown = true;
bool allow_going_over_size_limit = false;

const db::extensions * extensions = nullptr;
};
Expand Down
7 changes: 7 additions & 0 deletions db/hints/manager.cc
Expand Up @@ -341,6 +341,13 @@ future<db::commitlog> manager::end_point_hints_manager::add_store() noexcept {
// them when commitlog is re-created. This is expected to happen regularly
// during standard HH workload, so no need to print a warning about it.
cfg.warn_about_segments_left_on_disk_after_shutdown = false;
// Allow going over the configured size limit of the commitlog
// (resource_manager::max_hints_per_ep_size_mb). The commitlog will
// be more conservative with its disk usage when going over the limit.
// On the other hand, HH counts used space using the space_watchdog
// in resource_manager, so its redundant for the commitlog to apply
// a hard limit.
cfg.allow_going_over_size_limit = true;

return commitlog::create_commitlog(std::move(cfg)).then([this] (commitlog l) {
// add_store() is triggered every time hint files are forcefully flushed to I/O (every hints_flush_period).
Expand Down

0 comments on commit added53

Please sign in to comment.