[PATCH v1 06/56] view: do not use get_endpoint_for_host_id_if_known to check if a node is part of the topology

[Gleb Natapov]

Check directly in the topology instead.
---
db/view/view.cc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/db/view/view.cc b/db/view/view.cc
index dd68531e649..48b1091ab29 100644
--- a/db/view/view.cc
+++ b/db/view/view.cc
@@ -2671,7 +2671,7 @@ future<> view_builder::migrate_to_v2(locator::token_metadata_ptr tmptr, db::syst
// In the v1 table we may have left over rows that belong to nodes that were removed
// and we didn't clean them, so do that now.
auto host_id = row.get_as<utils::UUID>("host_id");
- if (!tmptr->get_endpoint_for_host_id_if_known(locator::host_id(host_id))) {
+ if (!tmptr->get_topology().find_node(locator::host_id(host_id))) {
vlogger.warn("Dropping a row from view_build_status: host {} does not exist", host_id);
continue;
}
@@ -3151,7 +3151,7 @@ future<bool> view_builder::check_view_build_ongoing(const locator::token_metadat
return view_status(ks_name, cf_name).then([&tm] (view_statuses_type&& view_statuses) {
return std::ranges::any_of(view_statuses, [&tm] (const view_statuses_type::value_type& view_status) {
// Only consider status of known hosts.
- return view_status.second == "STARTED" && tm.get_endpoint_for_host_id_if_known(view_status.first);
+ return view_status.second == "STARTED" && tm.get_topology().find_node(view_status.first);
});
});
}
--
2.47.1

[Kamil Braun]

On 1/13/25 9:06 AM, 'Gleb Natapov' via ScyllaDB development wrote:
> Check directly in the topology instead.

IIUC this changes the logic from considering only token owners (using
token metadata) to considering zero-token nodes too (using topology). Is
that correct?

Patryk, Dawid, Piotr, please review

[Gleb Natapov]

On Mon, Jan 13, 2025 at 04:31:58PM +0100, Kamil Braun wrote:
>
>
> On 1/13/25 9:06 AM, 'Gleb Natapov' via ScyllaDB development wrote:
> > Check directly in the topology instead.
> IIUC this changes the logic from considering only token owners (using token
> metadata) to considering zero-token nodes too (using topology). Is that
> correct?
>

As far as I see the change is absolutely identical. The
get_endpoint_for_host_id_if_known looked like that:

std::optional<inet_address> token_metadata_impl::get_endpoint_for_host_id_if_known(host_id host_id) const {
if (const auto* node = _topology.find_node(host_id)) [[likely]] {
return node->endpoint();
} else {
return std::nullopt;
}
}

So it returned unengaged optional if there was no node in the topology.
The patch uses _topology.find_node directly.

> Patryk, Dawid, Piotr, please review
>
> > ---
> > db/view/view.cc | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/db/view/view.cc b/db/view/view.cc
> > index dd68531e649..48b1091ab29 100644
> > --- a/db/view/view.cc
> > +++ b/db/view/view.cc
> > @@ -2671,7 +2671,7 @@ future<> view_builder::migrate_to_v2(locator::token_metadata_ptr tmptr, db::syst
> > // In the v1 table we may have left over rows that belong to nodes that were removed
> > // and we didn't clean them, so do that now.
> > auto host_id = row.get_as<utils::UUID>("host_id");
> > - if (!tmptr->get_endpoint_for_host_id_if_known(locator::host_id(host_id))) {
> > + if (!tmptr->get_topology().find_node(locator::host_id(host_id))) {
> > vlogger.warn("Dropping a row from view_build_status: host {} does not exist", host_id);
> > continue;
> > }
> > @@ -3151,7 +3151,7 @@ future<bool> view_builder::check_view_build_ongoing(const locator::token_metadat
> > return view_status(ks_name, cf_name).then([&tm] (view_statuses_type&& view_statuses) {
> > return std::ranges::any_of(view_statuses, [&tm] (const view_statuses_type::value_type& view_status) {
> > // Only consider status of known hosts.
> > - return view_status.second == "STARTED" && tm.get_endpoint_for_host_id_if_known(view_status.first);
> > + return view_status.second == "STARTED" && tm.get_topology().find_node(view_status.first);
> > });
> > });
> > }
>

--
Gleb.