mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-05-23 00:02:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

reader_concurrency_semaphore: remove default name parameter

Browse Source 
Naming the concurrency semaphore is currently optional, unnamed
semaphores defaulting to "Unnamed semaphore". Although the most
important semaphores are named, many still aren't, which makes for a
poor debugging experience when one of these times out.
To prevent this, remove the name parameter defaults from those
constructors that have it and require a unique name to be passed in.
Also update all sites creating a semaphore and make sure they use a
unique name.
This commit is contained in:
Botond Dénes
2021-06-11 14:20:20 +03:00
parent 1924e8d2b6
commit c4e71fb9b8
5 changed files with 14 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
database.cc
View File

@@ -338,7 +338,7 @@ database::database(const db::config& cfg, database_config dbcfg, service::migrat
max_memory_streaming_concurrent_reads(),
"_streaming_concurrency_sem")
// No limits, just for accounting.
, _compaction_concurrency_sem(reader_concurrency_semaphore::no_limits{})
, _compaction_concurrency_sem(reader_concurrency_semaphore::no_limits{}, "compaction")
, _system_read_concurrency_sem(
// Using higher initial concurrency, see revert_initial_system_read_concurrency_boost().
max_count_concurrent_reads,

6
reader_concurrency_semaphore.hh
View File

@@ -198,6 +198,9 @@ private:
public:
struct no_limits { };
/// Create a semaphore with the specified limits
///
/// The semaphore's name has to be unique!
reader_concurrency_semaphore(int count,
ssize_t memory,
sstring name,
@@ -207,7 +210,8 @@ public:
/// Create a semaphore with practically unlimited count and memory.
///
/// And conversely, no queue limit either.
explicit reader_concurrency_semaphore(no_limits, sstring name = "unlimited reader_concurrency_semaphore");
/// The semaphore's name has to be unique!
explicit reader_concurrency_semaphore(no_limits, sstring name);
~reader_concurrency_semaphore();

4
sstables/sstables.cc
View File

@@ -1774,7 +1774,7 @@ future<> sstable::generate_summary(const io_priority_class& pc) {
};
return new_sstable_component_file(_read_error_handler, component_type::Index, open_flags::ro).then([this, &pc] (file index_file) {
return do_with(std::move(index_file), std::make_unique<reader_concurrency_semaphore>(reader_concurrency_semaphore::no_limits{}),
return do_with(std::move(index_file), std::make_unique<reader_concurrency_semaphore>(reader_concurrency_semaphore::no_limits{}, "sstables::generate_summary()"),
[this, &pc] (file index_file, std::unique_ptr<reader_concurrency_semaphore>& sem) {
return index_file.size().then([this, &pc, index_file, &sem] (auto index_size) {
// an upper bound. Surely to be less than this.
@@ -2679,7 +2679,7 @@ future<bool> sstable::has_partition_key(const utils::hashed_key& hk, const dht::
if (!filter_has_key(hk)) {
return make_ready_future<bool>(false);
}
auto sem = std::make_unique<reader_concurrency_semaphore>(reader_concurrency_semaphore::no_limits{});
auto sem = std::make_unique<reader_concurrency_semaphore>(reader_concurrency_semaphore::no_limits{}, "sstables::has_partition_key()");
auto lh_index_ptr = std::make_unique<sstables::index_reader>(s, sem->make_permit(_schema.get(), s->get_filename()), default_priority_class(), tracing::trace_state_ptr());
auto& lh_index = *lh_index_ptr;
return lh_index.advance_lower_and_check_if_present(dk).then([lh_index_ptr = std::move(lh_index_ptr), s, sem = std::move(sem)] (bool present) mutable {

7
test/lib/reader_lifecycle_policy.hh
View File

@@ -90,11 +90,14 @@ public:
} else if (_contexts[shard]->semaphore) {
return *_contexts[shard]->semaphore;
}
// To support multiple reader life-cycle instances alive at the same
// time, incorporate `this` into the name, to make their names unique.
auto name = format("tests::reader_lifecycle_policy@{}@shard_id={}", fmt::ptr(this), shard);
if (_evict_paused_readers) {
// Create with no memory, so all inactive reads are immediately evicted.
_contexts[shard]->semaphore.emplace(1, 0, format("reader_concurrency_semaphore @shard_id={}", shard));
_contexts[shard]->semaphore.emplace(1, 0, std::move(name));
} else {
_contexts[shard]->semaphore.emplace(reader_concurrency_semaphore::no_limits{});
_contexts[shard]->semaphore.emplace(reader_concurrency_semaphore::no_limits{}, std::move(name));
}
return *_contexts[shard]->semaphore;
}

2
test/lib/reader_permit.cc
View File

@@ -24,7 +24,7 @@
namespace tests {
thread_local reader_concurrency_semaphore the_semaphore{reader_concurrency_semaphore::no_limits{}};
thread_local reader_concurrency_semaphore the_semaphore{reader_concurrency_semaphore::no_limits{}, "global_test_semaphore"};
reader_concurrency_semaphore& semaphore() {
return the_semaphore;