From d496741bb8e0139b8776648aa79e89812a0cfc25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Ma=C5=9Blanka?= <michal@redpanda.com>
Date: Wed, 28 Aug 2024 07:52:46 +0000
Subject: [PATCH] c/members_table: include members table version in the
 snapshot
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Members table version is updated only when cluster membership is
updated. The version must be included in the snapshot as it not always
equal to the snapshot last applied offset.

This PR makes the `members_table::version` consistent across the nodes
even if controller snapshot was applied. The version is exposed to end
users in `/v1/brokers/cluster_view` endpoint.

Fixes: CORE-6783

Signed-off-by: Michał Maślanka <michal@redpanda.com>
---
 src/v/cluster/controller_snapshot.h | 8 ++++++--
 src/v/cluster/members_table.cc      | 9 +++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/v/cluster/controller_snapshot.h b/src/v/cluster/controller_snapshot.h
index 99ccb07b6ba6f..ce4df42185dbd 100644
--- a/src/v/cluster/controller_snapshot.h
+++ b/src/v/cluster/controller_snapshot.h
@@ -53,7 +53,7 @@ struct features_t
 
 struct members_t
   : public serde::
-      envelope<members_t, serde::version<1>, serde::compat_version<0>> {
+      envelope<members_t, serde::version<2>, serde::compat_version<0>> {
     struct node_t
       : serde::envelope<node_t, serde::version<0>, serde::compat_version<0>> {
         model::broker broker;
@@ -86,6 +86,9 @@ struct members_t
     absl::node_hash_map<model::node_id, update_t> in_progress_updates;
 
     model::offset first_node_operation_command_offset;
+    // revision of metadata table (offset of last applied cluster member
+    // command)
+    model::revision_id version{};
 
     friend bool operator==(const members_t&, const members_t&) = default;
 
@@ -97,7 +100,8 @@ struct members_t
           removed_nodes,
           removed_nodes_still_in_raft0,
           in_progress_updates,
-          first_node_operation_command_offset);
+          first_node_operation_command_offset,
+          version);
     }
 };
 
diff --git a/src/v/cluster/members_table.cc b/src/v/cluster/members_table.cc
index 6859a48c29cdf..ffdba22c3e22c 100644
--- a/src/v/cluster/members_table.cc
+++ b/src/v/cluster/members_table.cc
@@ -293,14 +293,19 @@ void members_table::fill_snapshot(controller_snapshot& controller_snap) {
           controller_snapshot_parts::members_t::node_t{
             .broker = md.broker, .state = md.state});
     }
+    snap.version = _version;
 }
 
 void members_table::apply_snapshot(
   model::offset snap_offset, const controller_snapshot& controller_snap) {
-    _version = model::revision_id(snap_offset);
-
     const auto& snap = controller_snap.members;
 
+    // if version is present in snapshot use it, otherwise fallback to old
+    // behavior
+    _version = snap.version != model::revision_id{}
+                 ? snap.version
+                 : model::revision_id(snap_offset);
+
     // update the list of brokers
 
     cache_t old_nodes;