diff --git a/locator/abstract_replication_strategy.cc b/locator/abstract_replication_strategy.cc index 808c9f0338..d54c754be7 100644 --- a/locator/abstract_replication_strategy.cc +++ b/locator/abstract_replication_strategy.cc @@ -112,10 +112,10 @@ inet_address_vector_topology_change vnode_effective_replication_map::get_pending return endpoints; } -std::optional vnode_effective_replication_map::get_endpoints_for_reading(const token& token) const { +inet_address_vector_replica_set vnode_effective_replication_map::get_endpoints_for_reading(const token& token) const { const auto* endpoints = find_token(_read_endpoints, token); if (endpoints == nullptr) { - return {}; + return get_natural_endpoints_without_node_being_replaced(token); } return inet_address_vector_replica_set(endpoints->begin(), endpoints->end()); } diff --git a/locator/abstract_replication_strategy.hh b/locator/abstract_replication_strategy.hh index cf3eb16338..11a98ecfe9 100644 --- a/locator/abstract_replication_strategy.hh +++ b/locator/abstract_replication_strategy.hh @@ -201,9 +201,7 @@ public: virtual inet_address_vector_topology_change get_pending_endpoints(const token& search_token) const = 0; /// Returns a list of nodes to which a read request should be directed. - /// Returns not null only during topology changes, if request_read_new was called and - /// new set of replicas differs from the old one. - virtual std::optional get_endpoints_for_reading(const token& search_token) const = 0; + virtual inet_address_vector_replica_set get_endpoints_for_reading(const token& search_token) const = 0; /// Returns true if there are any pending ranges for this endpoint. /// This operation is expensive, for vnode_erm it iterates @@ -279,7 +277,7 @@ public: // effective_replication_map inet_address_vector_replica_set get_natural_endpoints(const token& search_token) const override; inet_address_vector_replica_set get_natural_endpoints_without_node_being_replaced(const token& search_token) const override; inet_address_vector_topology_change get_pending_endpoints(const token& search_token) const override; - std::optional get_endpoints_for_reading(const token& search_token) const override; + inet_address_vector_replica_set get_endpoints_for_reading(const token& search_token) const override; bool has_pending_ranges(inet_address endpoint) const override; std::unique_ptr make_splitter() const override; const dht::sharder& get_sharder(const schema& s) const override; diff --git a/locator/tablets.cc b/locator/tablets.cc index 3c066bea0d..bb7acdcc09 100644 --- a/locator/tablets.cc +++ b/locator/tablets.cc @@ -260,8 +260,8 @@ public: return {get_endpoint_for_host_id(info->pending_replica.host)}; } - virtual std::optional get_endpoints_for_reading(const token& search_token) const override { - return std::nullopt; + virtual inet_address_vector_replica_set get_endpoints_for_reading(const token& search_token) const override { + return get_natural_endpoints_without_node_being_replaced(search_token); } virtual bool has_pending_ranges(inet_address endpoint) const override { diff --git a/service/storage_proxy.cc b/service/storage_proxy.cc index 4376adf9f3..cf53b4d9a5 100644 --- a/service/storage_proxy.cc +++ b/service/storage_proxy.cc @@ -6184,13 +6184,10 @@ void storage_proxy::sort_endpoints_by_proximity(const locator::topology& topo, i inet_address_vector_replica_set storage_proxy::get_endpoints_for_reading(const sstring& ks_name, const locator::effective_replication_map& erm, const dht::token& token) const { auto endpoints = erm.get_endpoints_for_reading(token); - if (!endpoints) { - endpoints = erm.get_natural_endpoints_without_node_being_replaced(token); - } - auto it = boost::range::remove_if(*endpoints, std::not_fn(std::bind_front(&storage_proxy::is_alive, this))); - endpoints->erase(it, endpoints->end()); - sort_endpoints_by_proximity(erm.get_topology(), *endpoints); - return std::move(*endpoints); + auto it = boost::range::remove_if(endpoints, std::not_fn(std::bind_front(&storage_proxy::is_alive, this))); + endpoints.erase(it, endpoints.end()); + sort_endpoints_by_proximity(erm.get_topology(), endpoints); + return endpoints; } // `live_endpoints` must already contain only replicas for this query; the function only filters out some of them. diff --git a/test/boost/token_metadata_test.cc b/test/boost/token_metadata_test.cc index cccd34de3f..e1598d2b5e 100644 --- a/test/boost/token_metadata_test.cc +++ b/test/boost/token_metadata_test.cc @@ -43,31 +43,6 @@ namespace { } } -SEASTAR_THREAD_TEST_CASE(test_pending_and_read_endpoints_for_bootstrap_first_node) { - const auto e1 = inet_address("192.168.0.1"); - const auto t1 = dht::token::from_int64(1); - - auto token_metadata = create_token_metadata(e1); - token_metadata->update_topology(e1, get_dc_rack(e1)); - token_metadata->add_bootstrap_token(t1, e1); - - { - auto erm = create_erm(token_metadata, {{"replication_factor", "1"}}); - BOOST_REQUIRE_EQUAL(erm->get_pending_endpoints(dht::token::from_int64(0)), - inet_address_vector_topology_change{e1}); - BOOST_REQUIRE_EQUAL(erm->get_pending_endpoints(dht::token::from_int64(1)), - inet_address_vector_topology_change{e1}); - BOOST_REQUIRE_EQUAL(erm->get_pending_endpoints(dht::token::from_int64(2)), - inet_address_vector_topology_change{e1}); - BOOST_REQUIRE(!erm->get_endpoints_for_reading(t1).has_value()); - } - { - token_metadata->set_read_new(token_metadata::read_new_t::yes); - auto erm = create_erm(token_metadata, {{"replication_factor", "1"}}); - BOOST_REQUIRE_EQUAL(erm->get_endpoints_for_reading(t1), inet_address_vector_replica_set{e1}); - } -} - SEASTAR_THREAD_TEST_CASE(test_pending_and_read_endpoints_for_everywhere_strategy) { const auto e1 = inet_address("192.168.0.1"); const auto e2 = inet_address("192.168.0.2"); @@ -236,9 +211,8 @@ SEASTAR_THREAD_TEST_CASE(test_endpoints_for_reading_when_bootstrap_with_replicas const auto expected_set = std::unordered_set(expected_replicas.begin(), expected_replicas.end()); const auto actual_replicas = erm->get_endpoints_for_reading(dht::token::from_int64(t)); - BOOST_REQUIRE(actual_replicas.has_value()); - const auto actual_set = std::unordered_set(actual_replicas->begin(), - actual_replicas->end()); + const auto actual_set = std::unordered_set(actual_replicas.begin(), + actual_replicas.end()); BOOST_REQUIRE_EQUAL(expected_set, actual_set); }; @@ -246,7 +220,8 @@ SEASTAR_THREAD_TEST_CASE(test_endpoints_for_reading_when_bootstrap_with_replicas seastar::compat::source_location sl = seastar::compat::source_location::current()) { BOOST_TEST_INFO("line: " << sl.line()); - BOOST_REQUIRE(!erm->get_endpoints_for_reading(dht::token::from_int64(t)).has_value()); + BOOST_REQUIRE_EQUAL(erm->get_endpoints_for_reading(dht::token::from_int64(t)), + erm->get_natural_endpoints(dht::token::from_int64(t))); }; {