[PATCH v1 01/56] service: address_map: add lookup function that expects address to exist

[Gleb Natapov]

We will add code that expects id to ip mapping to exist. If it does not
it is better to fail earlier during testing, so add a function that
calls internal error in case there is no mapping.
---
service/address_map.hh | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/service/address_map.hh b/service/address_map.hh
index e13c01f0d59..c3716ed82ce 100644
--- a/service/address_map.hh
+++ b/service/address_map.hh
@@ -298,6 +298,15 @@ class address_map_t : public peering_sharded_service<address_map_t<Clock>> {
return entry._addr;
}

+ // Same as find() above but expects mapping to exist
+ gms::inet_address get(locator::host_id id) const {
+ try {
+ return find(id).value();
+ } catch (std::bad_optional_access& err) {
+ on_internal_error(rslog, fmt::format("No ip address for {} when one is expected", id));
+ }
+ }
+
// Find an id with a given mapping.
//
// If a mapping is expiring, the last access timestamp is updated automatically.
--
2.47.1

[Kamil Braun]

On 1/13/25 9:06 AM, 'Gleb Natapov' via ScyllaDB development wrote:
> We will add code that expects id to ip mapping to exist. If it does not
> it is better to fail earlier during testing, so add a function that
> calls internal error in case there is no mapping.
> ---
> service/address_map.hh | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/service/address_map.hh b/service/address_map.hh
> index e13c01f0d59..c3716ed82ce 100644
> --- a/service/address_map.hh
> +++ b/service/address_map.hh
> @@ -298,6 +298,15 @@ class address_map_t : public peering_sharded_service<address_map_t<Clock>> {
> return entry._addr;
> }
>
> + // Same as find() above but expects mapping to exist
> + gms::inet_address get(locator::host_id id) const {
> + try {
> + return find(id).value();
> + } catch (std::bad_optional_access& err) {

I suspect this code will be used often and potentially on hot path (?)
in which case would be better to use map_type::find directly and check
for end(), fewer branches that way (proposed code we'd check iterator !=
end(), wrap result to optional, then check the optional again with value())

[Gleb Natapov]

On Mon, Jan 13, 2025 at 04:31:18PM +0100, Kamil Braun wrote:
>
>
> On 1/13/25 9:06 AM, 'Gleb Natapov' via ScyllaDB development wrote:
> > We will add code that expects id to ip mapping to exist. If it does not
> > it is better to fail earlier during testing, so add a function that
> > calls internal error in case there is no mapping.
> > ---
> > service/address_map.hh | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/service/address_map.hh b/service/address_map.hh
> > index e13c01f0d59..c3716ed82ce 100644
> > --- a/service/address_map.hh
> > +++ b/service/address_map.hh
> > @@ -298,6 +298,15 @@ class address_map_t : public peering_sharded_service<address_map_t<Clock>> {
> > return entry._addr;
> > }
> > + // Same as find() above but expects mapping to exist
> > + gms::inet_address get(locator::host_id id) const {
> > + try {
> > + return find(id).value();
> > + } catch (std::bad_optional_access& err) {
> I suspect this code will be used often and potentially on hot path (?) in

No. It should not.

> which case would be better to use map_type::find directly and check for
> end(), fewer branches that way (proposed code we'd check iterator != end(),
> wrap result to optional, then check the optional again with value())
> > + on_internal_error(rslog, fmt::format("No ip address for {} when one is expected", id));
> > + }
> > + }
> > +
> > // Find an id with a given mapping.
> > //
> > // If a mapping is expiring, the last access timestamp is updated automatically.
>

--
Gleb.

[Gleb Natapov]

We will add code that expects id to ip mapping to exist. If it does not
it is better to fail earlier during testing, so add a function that
calls internal error in case there is no mapping.
---
service/address_map.hh | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/service/address_map.hh b/service/address_map.hh
index e13c01f0d59..c3716ed82ce 100644
--- a/service/address_map.hh
+++ b/service/address_map.hh
@@ -298,6 +298,15 @@ class address_map_t : public peering_sharded_service<address_map_t<Clock>> {
return entry._addr;
}

+ // Same as find() above but expects mapping to exist
+ gms::inet_address get(locator::host_id id) const {
+ try {
+ return find(id).value();
+ } catch (std::bad_optional_access& err) {

+ on_internal_error(rslog, fmt::format("No ip address for {} when one is expected", id));
+ }
+ }
+
// Find an id with a given mapping.
//
// If a mapping is expiring, the last access timestamp is updated automatically.
--

2.47.1