diff --git a/compatible_ring_position.hh b/compatible_ring_position.hh deleted file mode 100644 index 408bcdb987..0000000000 --- a/compatible_ring_position.hh +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (C) 2019-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - - -#pragma once - -#include "dht/ring_position.hh" - -// Wraps ring_position or ring_position_view so either is compatible with old-style C++: default -// constructor, stateless comparators, yada yada. -// The motivations for supporting both types are to make containers self-sufficient by not relying -// on callers to keep ring position alive, allow lookup on containers that don't support different -// key types, and also avoiding unnecessary copies. -class compatible_ring_position_or_view { - schema_ptr _schema; - lw_shared_ptr _rp; - dht::ring_position_view_opt _rpv; // optional only for default ctor, nothing more -public: - compatible_ring_position_or_view() = default; - explicit compatible_ring_position_or_view(schema_ptr s, dht::ring_position rp) - : _schema(std::move(s)), _rp(make_lw_shared(std::move(rp))), _rpv(dht::ring_position_view(*_rp)) { - } - explicit compatible_ring_position_or_view(const schema& s, dht::ring_position_view rpv) - : _schema(s.shared_from_this()), _rpv(rpv) { - } - const dht::ring_position_view& position() const { - return *_rpv; - } - std::strong_ordering operator<=>(const compatible_ring_position_or_view& other) const { - return dht::ring_position_tri_compare(*_schema, position(), other.position()); - } - bool operator==(const compatible_ring_position_or_view& other) const { - return *this <=> other == 0; - } -}; diff --git a/dht/ring_position.hh b/dht/ring_position.hh index 5bae7a3143..ce870b9591 100644 --- a/dht/ring_position.hh +++ b/dht/ring_position.hh @@ -261,7 +261,6 @@ public: friend class optimized_optional; }; -using ring_position_ext_view = ring_position_view; using ring_position_view_opt = optimized_optional; // @@ -447,6 +446,34 @@ struct ring_position_less_comparator { } }; +// Wraps ring_position or ring_position_view so either is compatible with old-style C++: default +// constructor, stateless comparators, yada yada. +// The motivations for supporting both types are to make containers self-sufficient by not relying +// on callers to keep ring position alive, allow lookup on containers that don't support different +// key types, and also avoiding unnecessary copies. +class compatible_ring_position_or_view { + schema_ptr _schema; + lw_shared_ptr _rp; + dht::ring_position_view_opt _rpv; // optional only for default ctor, nothing more +public: + compatible_ring_position_or_view() = default; + explicit compatible_ring_position_or_view(schema_ptr s, dht::ring_position rp) + : _schema(std::move(s)), _rp(make_lw_shared(std::move(rp))), _rpv(dht::ring_position_view(*_rp)) { + } + explicit compatible_ring_position_or_view(const schema& s, dht::ring_position_view rpv) + : _schema(s.shared_from_this()), _rpv(rpv) { + } + const dht::ring_position_view& position() const { + return *_rpv; + } + std::strong_ordering operator<=>(const compatible_ring_position_or_view& other) const { + return dht::ring_position_tri_compare(*_schema, position(), other.position()); + } + bool operator==(const compatible_ring_position_or_view& other) const { + return *this <=> other == 0; + } +}; + class partition_ranges_view { const dht::partition_range* _data = nullptr; size_t _size = 0; diff --git a/sstables/sstable_set.cc b/sstables/sstable_set.cc index 717fe7abb3..9f18d1aab1 100644 --- a/sstables/sstable_set.cc +++ b/sstables/sstable_set.cc @@ -15,7 +15,7 @@ #include "sstables.hh" -#include "compatible_ring_position.hh" +#include "dht/ring_position.hh" #include "compaction/compaction_strategy_impl.hh" #include "compaction/leveled_compaction_strategy.hh" #include "compaction/time_window_compaction_strategy.hh" @@ -222,8 +222,8 @@ sstable_set::make_incremental_selector() const { partitioned_sstable_set::interval_type partitioned_sstable_set::make_interval(const schema& s, const dht::partition_range& range) { return interval_type::closed( - compatible_ring_position_or_view(s, dht::ring_position_view(range.start()->value())), - compatible_ring_position_or_view(s, dht::ring_position_view(range.end()->value()))); + dht::compatible_ring_position_or_view(s, dht::ring_position_view(range.start()->value())), + dht::compatible_ring_position_or_view(s, dht::ring_position_view(range.end()->value()))); } partitioned_sstable_set::interval_type partitioned_sstable_set::make_interval(const dht::partition_range& range) const { @@ -232,8 +232,8 @@ partitioned_sstable_set::interval_type partitioned_sstable_set::make_interval(co partitioned_sstable_set::interval_type partitioned_sstable_set::make_interval(const schema_ptr& s, const sstable& sst) { return interval_type::closed( - compatible_ring_position_or_view(s, dht::ring_position(sst.get_first_decorated_key())), - compatible_ring_position_or_view(s, dht::ring_position(sst.get_last_decorated_key()))); + dht::compatible_ring_position_or_view(s, dht::ring_position(sst.get_first_decorated_key())), + dht::compatible_ring_position_or_view(s, dht::ring_position(sst.get_last_decorated_key()))); } partitioned_sstable_set::interval_type partitioned_sstable_set::make_interval(const sstable& sst) { @@ -243,7 +243,7 @@ partitioned_sstable_set::interval_type partitioned_sstable_set::make_interval(co partitioned_sstable_set::interval_type partitioned_sstable_set::singular(const dht::ring_position& rp) const { // We should use the view here, since this is used for queries. auto rpv = dht::ring_position_view(rp); - auto crp = compatible_ring_position_or_view(*_schema, std::move(rpv)); + auto crp = dht::compatible_ring_position_or_view(*_schema, std::move(rpv)); return interval_type::closed(crp, crp); } @@ -267,7 +267,7 @@ bool partitioned_sstable_set::store_as_unleveled(const shared_sstable& sst) cons return _use_level_metadata && sst->get_sstable_level() == 0; } -dht::ring_position partitioned_sstable_set::to_ring_position(const compatible_ring_position_or_view& crp) { +dht::ring_position partitioned_sstable_set::to_ring_position(const dht::compatible_ring_position_or_view& crp) { // Ring position views, representing bounds of sstable intervals are // guaranteed to have key() != nullptr; const auto& pos = crp.position(); @@ -432,14 +432,14 @@ private: return dht::ring_position_ext(next_position, dht::ring_position_ext::after_key(!boost::icl::is_left_closed(it->first.bounds()))); } } - static bool is_before_interval(const compatible_ring_position_or_view& crp, const interval_type& interval) { + static bool is_before_interval(const dht::compatible_ring_position_or_view& crp, const interval_type& interval) { if (boost::icl::is_left_closed(interval.bounds())) { return crp < interval.lower(); } else { return crp <= interval.lower(); } } - void maybe_invalidate_iterator(const compatible_ring_position_or_view& crp) { + void maybe_invalidate_iterator(const dht::compatible_ring_position_or_view& crp) { if (_last_known_leveled_sstables_change_cnt != _leveled_sstables_change_cnt) { _it = _leveled_sstables.lower_bound(interval_type::closed(crp, crp)); _last_known_leveled_sstables_change_cnt = _leveled_sstables_change_cnt; @@ -456,7 +456,7 @@ public: , _it(leveled_sstables.begin()) { } virtual std::tuple, dht::ring_position_ext> select(const dht::ring_position_view& pos) override { - auto crp = compatible_ring_position_or_view(*_schema, pos); + auto crp = dht::compatible_ring_position_or_view(*_schema, pos); auto ssts = _unleveled_sstables; using namespace dht; diff --git a/sstables/sstable_set_impl.hh b/sstables/sstable_set_impl.hh index d12012d1c2..c937be69b3 100644 --- a/sstables/sstable_set_impl.hh +++ b/sstables/sstable_set_impl.hh @@ -10,7 +10,7 @@ #include -#include "compatible_ring_position.hh" +#include "dht/ring_position.hh" #include "sstable_set.hh" #include "readers/clustering_combined.hh" #include "sstables/types_fwd.hh" @@ -21,7 +21,7 @@ namespace sstables { // e.g. leveled compaction strategy class partitioned_sstable_set : public sstable_set_impl { using value_set = std::unordered_set; - using interval_map_type = boost::icl::interval_map; + using interval_map_type = boost::icl::interval_map; using interval_type = interval_map_type::interval_type; using map_iterator = interval_map_type::const_iterator; private: @@ -44,7 +44,7 @@ private: // SSTables are stored separately to avoid interval map's fragmentation issue when level 0 falls behind. bool store_as_unleveled(const shared_sstable& sst) const; public: - static dht::ring_position to_ring_position(const compatible_ring_position_or_view& crp); + static dht::ring_position to_ring_position(const dht::compatible_ring_position_or_view& crp); static dht::partition_range to_partition_range(const interval_type& i); static dht::partition_range to_partition_range(const dht::ring_position_view& pos, const interval_type& i); diff --git a/test/boost/sstable_datafile_test.cc b/test/boost/sstable_datafile_test.cc index dcc9738262..2bdae26766 100644 --- a/test/boost/sstable_datafile_test.cc +++ b/test/boost/sstable_datafile_test.cc @@ -33,7 +33,7 @@ #include "test/lib/index_reader_assertions.hh" #include "test/lib/make_random_string.hh" #include "test/lib/simple_schema.hh" -#include "compatible_ring_position.hh" +#include "dht/ring_position.hh" #include "partition_slice_builder.hh" #include "replica/memtable-sstable.hh" @@ -2552,7 +2552,7 @@ static dht::token token_from_long(int64_t value) { SEASTAR_TEST_CASE(basic_interval_map_testing_for_sstable_set) { using value_set = std::unordered_set; - using interval_map_type = boost::icl::interval_map; + using interval_map_type = boost::icl::interval_map; using interval_type = interval_map_type::interval_type; interval_map_type map; @@ -2562,8 +2562,8 @@ SEASTAR_TEST_CASE(basic_interval_map_testing_for_sstable_set) { .with_column("value", int32_type); auto s = builder.build(); - auto make_pos = [&] (int64_t token) -> compatible_ring_position_or_view { - return compatible_ring_position_or_view(s, dht::ring_position::starting_at(token_from_long(token))); + auto make_pos = [&] (int64_t token) { + return dht::compatible_ring_position_or_view(s, dht::ring_position::starting_at(token_from_long(token))); }; auto add = [&] (int64_t start, int64_t end, int gen) {