id_allocator: make it return a result<...> instead of a homemade type

That's mostly for later use in raft_fixture::retry_with_leader
redpanda-data · May 10, 2024 · 07e714f · 07e714f
1 parent 94e99f0
commit 07e714f
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 29 deletions.
diff --git a/src/v/cluster/id_allocator_frontend.cc b/src/v/cluster/id_allocator_frontend.cc
@@ -121,11 +121,11 @@ reset_id_handler::process(ss::shard_id shard, reset_id_allocator_request req) {
           }
           return stm->reset_next_id(id, timeout)
             .then([](id_allocator_stm::stm_allocation_result r) {
-                if (r.raft_status != raft::errc::success) {
+                if (!r) {
                     vlog(
                       clusterlog.warn,
                       "allocate id stm call failed with {}",
-                      raft::make_error_code(r.raft_status).message());
+                      r.assume_error().message());
                     return reset_id_allocator_reply{errc::replication_error};
                 }
 
@@ -146,7 +146,7 @@ allocate_id_handler::process(ss::shard_id shard, allocate_id_request req) {
                 "can't get partition by {} ntp",
                 model::id_allocator_ntp);
               return ss::make_ready_future<allocate_id_reply>(
-                allocate_id_reply{0, errc::topic_not_exists});
+                0, errc::topic_not_exists);
           }
           auto stm = partition->id_allocator_stm();
           if (!stm) {
@@ -155,19 +155,19 @@ allocate_id_handler::process(ss::shard_id shard, allocate_id_request req) {
                 "can't get id allocator stm of the {}' partition",
                 model::id_allocator_ntp);
               return ss::make_ready_future<allocate_id_reply>(
-                allocate_id_reply{0, errc::topic_not_exists});
+                0, errc::topic_not_exists);
           }
           return stm->allocate_id(timeout).then(
             [](id_allocator_stm::stm_allocation_result r) {
-                if (r.raft_status != raft::errc::success) {
+                if (!r) {
                     vlog(
                       clusterlog.warn,
                       "allocate id stm call failed with {}",
-                      raft::make_error_code(r.raft_status).message());
-                    return allocate_id_reply{r.id, errc::replication_error};
+                      r.assume_error().message());
+                    return allocate_id_reply{-1, errc::replication_error};
                 }
 
-                return allocate_id_reply{r.id, errc::success};
+                return allocate_id_reply{r.assume_value(), errc::success};
             });
       });
 }

diff --git a/src/v/cluster/id_allocator_stm.cc b/src/v/cluster/id_allocator_stm.cc
@@ -76,27 +76,28 @@ id_allocator_stm::reset_next_id(
     return _lock
       .with(
         timeout, [this, id, timeout]() { return advance_state(id, timeout); })
-      .handle_exception_type([](const ss::semaphore_timed_out&) {
-          return stm_allocation_result{-1, raft::errc::timeout};
-      });
+      .handle_exception_type(
+        [](const ss::semaphore_timed_out&) -> stm_allocation_result {
+            return raft::make_error_code(raft::errc::timeout);
+        });
 }
 
 ss::future<id_allocator_stm::stm_allocation_result>
 id_allocator_stm::advance_state(
   int64_t value, model::timeout_clock::duration timeout) {
     if (!co_await sync(timeout)) {
-        co_return stm_allocation_result{-1, raft::errc::timeout};
+        co_return raft::make_error_code(raft::errc::timeout);
     }
     if (value < _curr_id) {
-        co_return stm_allocation_result{_curr_id, raft::errc::success};
+        co_return _curr_id;
     }
     _curr_id = value;
     auto success = co_await set_state(_curr_id + _batch_size, timeout);
     if (!success) {
-        co_return stm_allocation_result{-1, raft::errc::timeout};
+        co_return raft::make_error_code(raft::errc::timeout);
     }
     _curr_batch = _batch_size;
-    co_return stm_allocation_result{_curr_id, raft::errc::success};
+    co_return stm_allocation_result(_curr_id);
 }
 
 ss::future<bool> id_allocator_stm::set_state(
@@ -123,31 +124,32 @@ ss::future<id_allocator_stm::stm_allocation_result>
 id_allocator_stm::allocate_id(model::timeout_clock::duration timeout) {
     return _lock
       .with(timeout, [this, timeout]() { return do_allocate_id(timeout); })
-      .handle_exception_type([](const ss::semaphore_timed_out&) {
-          return stm_allocation_result{-1, raft::errc::timeout};
-      });
+      .handle_exception_type(
+        [](const ss::semaphore_timed_out&) -> stm_allocation_result {
+            return raft::make_error_code(raft::errc::timeout);
+        });
 }
 
 ss::future<id_allocator_stm::stm_allocation_result>
 id_allocator_stm::do_allocate_id(model::timeout_clock::duration timeout) {
     if (!co_await sync(timeout)) {
-        co_return stm_allocation_result{-1, raft::errc::timeout};
+        co_return raft::make_error_code(raft::errc::timeout);
     }
 
     if (_curr_batch == 0) {
         _curr_id = _state;
         if (!co_await set_state(_curr_id + _batch_size, timeout)) {
-            co_return stm_allocation_result{-1, raft::errc::timeout};
+            co_return raft::make_error_code(raft::errc::timeout);
         }
         _curr_batch = _batch_size;
     }
 
-    auto id = _curr_id;
+    int64_t id = _curr_id;
 
     _curr_id += 1;
     _curr_batch -= 1;
 
-    co_return stm_allocation_result{id, raft::errc::success};
+    co_return stm_allocation_result{id};
 }
 
 ss::future<> id_allocator_stm::apply(const model::record_batch& b) {

diff --git a/src/v/cluster/id_allocator_stm.h b/src/v/cluster/id_allocator_stm.h
@@ -34,10 +34,7 @@ class id_allocator_stm final : public raft::persisted_stm<> {
 public:
     static constexpr std::string_view name = "id_allocator_stm";
 
-    struct stm_allocation_result {
-        int64_t id;
-        raft::errc raft_status{raft::errc::success};
-    };
+    using stm_allocation_result = result<int64_t>;
 
     explicit id_allocator_stm(ss::logger&, raft::consensus*);
 

diff --git a/src/v/cluster/tests/id_allocator_stm_test.cc b/src/v/cluster/tests/id_allocator_stm_test.cc
@@ -60,10 +60,10 @@ struct id_allocator_stm_fixture : simple_raft_fixture {
         for (int i = 0; i < n; i++) {
             auto result = _stm->allocate_id(1s).get0();
 
-            BOOST_REQUIRE_EQUAL(raft::errc::success, result.raft_status);
-            BOOST_REQUIRE_LT(cur_last_id, result.id);
+            BOOST_REQUIRE_EQUAL(result.has_value(), true);
+            BOOST_REQUIRE_LT(cur_last_id, result.assume_value());
 
-            cur_last_id = result.id;
+            cur_last_id = result.assume_value();
         }
         return cur_last_id;
     }