From 5ad9d38cfcb2165aca291b11b0097a9fd84341f3 Mon Sep 17 00:00:00 2001
From: Georgy Moshkin <gmoshkin@picodata.io>
Date: Mon, 12 Aug 2024 22:13:12 +0300
Subject: [PATCH] fix: remove redundant UpdateTopology raft op

---
 src/plugin/mod.rs | 14 ++++++++++++--
 src/traft/node.rs | 10 ----------
 src/traft/op.rs   |  7 +------
 3 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs
index 38b2dc1c62..0a6d2d2da1 100644
--- a/src/plugin/mod.rs
+++ b/src/plugin/mod.rs
@@ -970,18 +970,28 @@ fn update_tier(upd_op: TopologyUpdateOp, timeout: Duration) -> traft::Result<()>
 
     let ident = upd_op.plugin_identity();
     let ranges = vec![
+        // Fail if someone updates this plugin record
         Range::new(ClusterwideTable::Plugin).eq([&ident.name, &ident.version]),
+        // Fail if someone updates this service record
         Range::new(ClusterwideTable::Service).eq([
             &ident.name,
             upd_op.service_name(),
             &ident.version,
         ]),
+        // Fail if someone proposes another plugin operation
+        Range::new(ClusterwideTable::Property).eq([PropertyName::PendingPluginOperation]),
     ];
-    let op = PluginRaftOp::UpdateServiceTopology { op: upd_op.clone() };
+
+    let op = PluginOp::UpdateTopology(upd_op.clone());
+    let dml = Dml::replace(
+        ClusterwideTable::Property,
+        &(&PropertyName::PendingPluginOperation, &op),
+        effective_user_id(),
+    )?;
 
     let mut index = do_plugin_cas(
         node,
-        Op::Plugin(op),
+        Op::Dml(dml),
         ranges,
         Some(|node| Ok(node.storage.properties.pending_plugin_op()?.is_some())),
         deadline,
diff --git a/src/traft/node.rs b/src/traft/node.rs
index ab284d6e4a..0fd8813103 100644
--- a/src/traft/node.rs
+++ b/src/traft/node.rs
@@ -734,7 +734,6 @@ impl NodeImpl {
             Op::DdlPrepare { .. } => true,
             // TODO: remove this once PluginOp::DisablePlugin is removed
             Op::Plugin(PluginRaftOp::DisablePlugin { .. }) => true,
-            Op::Plugin(PluginRaftOp::UpdateServiceTopology { .. }) => true,
             _ => false,
         };
 
@@ -1344,15 +1343,6 @@ impl NodeImpl {
                 }
             }
 
-            // TODO: remove this, just propose a DML into _pico_property directly
-            Op::Plugin(PluginRaftOp::UpdateServiceTopology { op }) => {
-                let plugin_op = PluginOp::UpdateTopology(op);
-                self.storage
-                    .properties
-                    .put(PropertyName::PendingPluginOperation, &plugin_op)
-                    .expect("storage should not fail");
-            }
-
             Op::Acl(acl) => {
                 let v_local = local_schema_version().expect("storage should not fail");
                 let v_pending = acl.schema_version();
diff --git a/src/traft/op.rs b/src/traft/op.rs
index f540948df0..3cb317cef2 100644
--- a/src/traft/op.rs
+++ b/src/traft/op.rs
@@ -1,4 +1,4 @@
-use crate::plugin::{PluginIdentifier, TopologyUpdateOp};
+use crate::plugin::PluginIdentifier;
 use crate::schema::{
     Distribution, IndexOption, PrivilegeDef, RoutineLanguage, RoutineParams, RoutineSecurity,
     UserDef, ADMIN_ID, GUEST_ID, PUBLIC_ID, SUPER_ID,
@@ -231,9 +231,6 @@ impl std::fmt::Display for Op {
             Op::Plugin(PluginRaftOp::RemovePlugin { ident }) => {
                 write!(f, "RemovePlugin({ident})")
             }
-            Op::Plugin(PluginRaftOp::UpdateServiceTopology { op }) => {
-                write!(f, "UpdateServiceTopology({op:?})",)
-            }
         };
 
         struct DisplayDml<'a>(&'a Dml);
@@ -821,8 +818,6 @@ pub enum PluginRaftOp {
     DisablePlugin { ident: PluginIdentifier },
     /// Remove selected plugin.
     RemovePlugin { ident: PluginIdentifier },
-    /// Update topology of a plugin service.
-    UpdateServiceTopology { op: TopologyUpdateOp },
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-- 
GitLab