From 589c7476e03dee7ac6866c7717f5b0665175f645 Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Tue, 10 Sep 2019 13:12:02 +0200 Subject: [PATCH 1/6] sstables: Move constructor out of line --- sstables/sstables.cc | 21 +++++++++++++++++++++ sstables/sstables.hh | 13 +------------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/sstables/sstables.cc b/sstables/sstables.cc index 82b05b412e..d0050470d2 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -3310,6 +3310,27 @@ mutation_source sstable::as_mutation_source() { }); } +sstable::sstable(schema_ptr schema, + sstring dir, + int64_t generation, + version_types v, + format_types f, + db::large_data_handler& large_data_handler, + gc_clock::time_point now, + io_error_handler_gen error_handler_gen, + size_t buffer_size) + : sstable_buffer_size(buffer_size) + , _schema(std::move(schema)) + , _dir(std::move(dir)) + , _generation(generation) + , _version(v) + , _format(f) + , _now(now) + , _read_error_handler(error_handler_gen(sstable_read_error)) + , _write_error_handler(error_handler_gen(sstable_write_error)) + , _large_data_handler(large_data_handler) +{ } + bool supports_correct_non_compound_range_tombstones() { return service::get_local_storage_service().cluster_supports_reading_correctly_serialized_range_tombstones(); } diff --git a/sstables/sstables.hh b/sstables/sstables.hh index 54f1ab8b51..1f97cf1397 100644 --- a/sstables/sstables.hh +++ b/sstables/sstables.hh @@ -137,18 +137,7 @@ public: db::large_data_handler& large_data_handler, gc_clock::time_point now, io_error_handler_gen error_handler_gen, - size_t buffer_size) - : sstable_buffer_size(buffer_size) - , _schema(std::move(schema)) - , _dir(std::move(dir)) - , _generation(generation) - , _version(v) - , _format(f) - , _now(now) - , _read_error_handler(error_handler_gen(sstable_read_error)) - , _write_error_handler(error_handler_gen(sstable_write_error)) - , _large_data_handler(large_data_handler) - { } + size_t buffer_size); sstable& operator=(const sstable&) = delete; sstable(const sstable&) = delete; sstable(sstable&&) = default; From fd74504e8755d53be17d91362c66ce3f3fb294d7 Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Tue, 10 Sep 2019 13:12:20 +0200 Subject: [PATCH 2/6] sstables: Make sstable object not movable Will be easier to add non-movable fields. We don't really need it to be movable, all instances should be managed by a shared pointer. --- sstables/sstables.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sstables/sstables.hh b/sstables/sstables.hh index 1f97cf1397..bf4bf7a314 100644 --- a/sstables/sstables.hh +++ b/sstables/sstables.hh @@ -140,7 +140,7 @@ public: size_t buffer_size); sstable& operator=(const sstable&) = delete; sstable(const sstable&) = delete; - sstable(sstable&&) = default; + sstable(sstable&&) = delete; ~sstable(); From 33bef82f6bb30e6623b84a18a2ac245f3292042a Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Tue, 10 Sep 2019 15:19:27 +0200 Subject: [PATCH 3/6] sstables: Track all instances of sstable objects Will make it easier to collect statistics about sstable in-memory metadata. --- sstables/sstables.cc | 18 +++++++++++++++++- sstables/sstables.hh | 6 ++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/sstables/sstables.cc b/sstables/sstables.cc index d0050470d2..9d0e2768fc 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -84,6 +84,20 @@ namespace sstables { logging::logger sstlog("sstable"); +namespace bi = boost::intrusive; + +class sstable_tracker { + bi::list, + bi::constant_time_size> _sstables; +public: + void add(sstable& sst) { + _sstables.push_back(sst); + } +}; + +static thread_local sstable_tracker tracker; + // Because this is a noop and won't hold any state, it is better to use a global than a // thread_local. It will be faster, specially on non-x86. static noop_write_monitor default_noop_write_monitor; @@ -3329,7 +3343,9 @@ sstable::sstable(schema_ptr schema, , _read_error_handler(error_handler_gen(sstable_read_error)) , _write_error_handler(error_handler_gen(sstable_write_error)) , _large_data_handler(large_data_handler) -{ } +{ + tracker.add(*this); +} bool supports_correct_non_compound_range_tombstones() { return service::get_local_storage_service().cluster_supports_reading_correctly_serialized_range_tombstones(); diff --git a/sstables/sstables.hh b/sstables/sstables.hh index bf4bf7a314..b7175b6f28 100644 --- a/sstables/sstables.hh +++ b/sstables/sstables.hh @@ -64,6 +64,7 @@ #include "sstables/shareable_components.hh" #include +#include class sstable_assertions; @@ -123,11 +124,15 @@ struct sstable_writer_config { utils::UUID run_identifier = utils::make_random_uuid(); }; +class sstable_tracker; + class sstable : public enable_lw_shared_from_this { friend ::sstable_assertions; + friend sstable_tracker; public: using version_types = sstable_version_types; using format_types = sstable_format_types; + using tracker_link_type = bi::list_member_hook>; public: sstable(schema_ptr schema, sstring dir, @@ -531,6 +536,7 @@ private: db::large_data_handler& _large_data_handler; sstables_stats _stats; + tracker_link_type _tracker_link; public: const bool has_component(component_type f) const; From c014c79d4b1276b264ccafe32c4381bcc2925de4 Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Tue, 10 Sep 2019 17:01:56 +0200 Subject: [PATCH 4/6] sstables: Track whether sstable was already open or not Some sstable objects correspond to sstables which are being written and are not sealed yet. Such sstables don't have all the fields filled-in. Tools which calculate statistics (like GDB scripts) need to distinguish such sstables. --- sstables/sstables.cc | 1 + sstables/sstables.hh | 1 + 2 files changed, 2 insertions(+) diff --git a/sstables/sstables.cc b/sstables/sstables.cc index 9d0e2768fc..77c3cf120b 100644 --- a/sstables/sstables.cc +++ b/sstables/sstables.cc @@ -1269,6 +1269,7 @@ future<> sstable::open_data() { if (_shards.empty()) { _shards = compute_shards_for_this_sstable(); } + _open = true; }); } diff --git a/sstables/sstables.hh b/sstables/sstables.hh index b7175b6f28..67275aee9b 100644 --- a/sstables/sstables.hh +++ b/sstables/sstables.hh @@ -466,6 +466,7 @@ private: foreign_ptr> _components = make_foreign(make_lw_shared()); column_translation _column_translation; bool _shared = true; // across shards; safe default + bool _open = false; // NOTE: _collector and _c_stats are used to generation of statistics file // when writing a new sstable. metadata_collector _collector; From a141d30eca82bc3ead3b522effa44e53a1c48581 Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Tue, 10 Sep 2019 15:20:37 +0200 Subject: [PATCH 5/6] gdb: Make intrusive_list recognize member_hook links GDB now gives "struct boost::intrusive::member_hook" from template_arguments() --- scylla-gdb.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scylla-gdb.py b/scylla-gdb.py index 96324e9ab0..86aa7943b7 100644 --- a/scylla-gdb.py +++ b/scylla-gdb.py @@ -50,6 +50,8 @@ class intrusive_list: # Some boost versions have this instead self.root = rps['m_header'] member_hook = get_template_arg_with_prefix(list_type, "boost::intrusive::member_hook") + if not member_hook: + member_hook = get_template_arg_with_prefix(list_type, "struct boost::intrusive::member_hook") if member_hook: self.link_offset = member_hook.template_argument(2).cast(self.size_t) else: From 06154569d56c033b43488be50d57e2cf1354d50f Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Tue, 10 Sep 2019 15:22:14 +0200 Subject: [PATCH 6/6] gdb: Use sstable tracker to get the list of sstables --- scylla-gdb.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/scylla-gdb.py b/scylla-gdb.py index 86aa7943b7..2e00808f4c 100644 --- a/scylla-gdb.py +++ b/scylla-gdb.py @@ -2219,16 +2219,8 @@ class scylla_cache(gdb.Command): def find_sstables(): """A generator which yields pointers to all live sstable objects on current shard.""" - visited = set() - # FIXME: Add support for other sstable sets. Also, we should change Scylla to make this easier - for sst_set in find_instances('sstables::bag_sstable_set'): - sstables = std_vector(sst_set['_sstables']) - for sst_ptr in sstables: - sst = seastar_lw_shared_ptr(sst_ptr).get() - if not int(sst) in visited: - visited.add(int(sst)) - yield sst - + for sst in intrusive_list(gdb.parse_and_eval('sstables::tracker._sstables')): + yield sst.address class scylla_sstables(gdb.Command): """Lists all sstable objects on currents shard together with useful information like on-disk and in-memory size.""" @@ -2244,6 +2236,8 @@ class scylla_sstables(gdb.Command): count = 0 for sst in find_sstables(): + if not sst['_open']: + continue count += 1 size = 0