Merge #1570

1570: feat(multireplicasnapshot): remove multi replica snapshot checks in dataplane r=hrudaya21 a=hrudaya21



Co-authored-by: Hrudaya <hrudayaranjan.sahoo@datacore.com>

Author: mayastor-bors

io-engine-tests/src/nexus.rs

@@ -19,6 +19,7 @@ use super::{
             RemoveChildNexusRequest,
             ShutdownNexusRequest,
         },
+        snapshot::SnapshotInfo,
         SharedRpcHandle,
         Status,
     },
@@ -33,10 +34,21 @@ use super::{
     },
     replica::ReplicaBuilder,
 };
-use io_engine::{constants::NVME_NQN_PREFIX, subsys::make_subsystem_serial};
+use io_engine::{
+    constants::NVME_NQN_PREFIX,
+    core::{SnapshotDescriptor, SnapshotParams},
+    subsys::make_subsystem_serial,
+};
 use std::time::{Duration, Instant};
 use tonic::Code;
 
+use io_engine_api::v1::snapshot::{
+    ListSnapshotsRequest,
+    NexusCreateSnapshotReplicaDescriptor,
+    NexusCreateSnapshotReplicaStatus,
+    NexusCreateSnapshotRequest,
+};
+
 #[derive(Clone)]
 pub struct NexusBuilder {
     rpc: SharedRpcHandle,
@@ -264,6 +276,42 @@ impl NexusBuilder {
         self.add_child(&self.replica_uri(r), norebuild).await
     }
 
+    pub async fn create_nexus_snapshot(
+        &self,
+        params: &SnapshotParams,
+        replicas: &[NexusCreateSnapshotReplicaDescriptor],
+    ) -> Result<Vec<NexusCreateSnapshotReplicaStatus>, Status> {
+        self.rpc()
+            .lock()
+            .await
+            .snapshot
+            .create_nexus_snapshot(NexusCreateSnapshotRequest {
+                nexus_uuid: self.uuid(),
+                entity_id: params.entity_id().unwrap(),
+                txn_id: params.txn_id().unwrap(),
+                snapshot_name: params.name().unwrap(),
+                replicas: replicas.to_vec(),
+            })
+            .await
+            .map(|r| r.into_inner().replicas_done)
+    }
+
+    pub async fn list_snapshot(&self) -> Vec<SnapshotInfo> {
+        self.rpc()
+            .lock()
+            .await
+            .snapshot
+            .list_snapshot(ListSnapshotsRequest {
+                source_uuid: None,
+                snapshot_uuid: None,
+                query: None,
+            })
+            .await
+            .map(|s| s.into_inner().snapshots)
+            .ok()
+            .unwrap()
+    }
+
     pub async fn remove_child_bdev(&self, bdev: &str) -> Result<Nexus, Status> {
         self.rpc()
             .lock()

io-engine/src/bdev/nexus/nexus_bdev_snapshot.rs

@@ -267,24 +267,13 @@ impl ReplicaSnapshotExecutor {
 impl<'n> Nexus<'n> {
     fn check_nexus_state(&self) -> Result<(), Error> {
         self.check_nexus_operation(NexusOperation::NexusSnapshot)?;
-
-        // Check that nexus has exactly 1 replica.
-        match self.children().len() {
-            0 => {
-                return Err(Error::FailedCreateSnapshot {
-                    name: self.bdev_name(),
-                    reason: "Nexus has no replicas".to_string(),
-                })
-            }
-            1 => {} // Only one replica nexuses are supported.
-            _ => {
-                return Err(Error::FailedCreateSnapshot {
-                    name: self.bdev_name(),
-                    reason: "Nexus has more than one replica".to_string(),
-                })
-            }
+        // Check that nexus has no children.
+        if self.children().is_empty() {
+            return Err(Error::FailedCreateSnapshot {
+                name: self.bdev_name(),
+                reason: "Nexus has no replicas".to_string(),
+            });
         }
-
         // Check that nexus is healthy and not being reconfigured.
         let state = *self.state.lock();
         if state != NexusState::Open {
@@ -332,14 +321,6 @@ impl<'n> Nexus<'n> {
 
         self.check_nexus_state()?;
 
-        // For now only single replica nexus is supported.
-        if self.children_iter().len() != 1 {
-            return Err(Error::FailedCreateSnapshot {
-                name: self.bdev_name(),
-                reason: "Nexus has more than one replica".to_string(),
-            });
-        }
-
         // Step 1: Pause I/O subsystem for nexus.
         self.as_mut().pause().await.map_err(|error| {
             error!(

io-engine/src/bin/io-engine-client/v1/snapshot_cli.rs

@@ -172,7 +172,9 @@ pub fn subcommands() -> Command {
         .subcommand(create_clone)
         .subcommand(list_clone)
 }
-
+/// For multiple replicas, replica_uuid will be given in a single string,
+/// separated by comma. Same for snapshot_uuid. replica_uuid and snapshot_uuid
+/// will be matched by index.
 async fn create_for_nexus(
     mut ctx: Context,
     matches: &ArgMatches,
@@ -202,24 +204,32 @@ async fn create_for_nexus(
         })?
         .to_owned();
     let replica_uuid = matches
-        .get_many::<String>("replica_uuid")
+        .get_one::<String>("replica_uuid")
         .ok_or_else(|| ClientError::MissingValue {
             field: "replica_uuid".to_string(),
         })?
-        .map(|c| c.to_string())
-        .collect::<Vec<String>>();
+        .to_owned();
     let snapshot_uuid = matches
-        .get_many::<String>("snapshot_uuid")
+        .get_one::<String>("snapshot_uuid")
         .ok_or_else(|| ClientError::MissingValue {
             field: "snapshot_uuid".to_string(),
         })?
-        .cloned()
-        .collect::<Vec<_>>();
+        .to_owned();
+
+    let replica_uuid: Vec<String> = replica_uuid
+        .split(',')
+        .map(|r| r.trim().to_string())
+        .collect();
+    let snapshot_uuid: Vec<String> = snapshot_uuid
+        .split(',')
+        .map(|s| s.trim().to_string())
+        .collect();
     if replica_uuid.len() != snapshot_uuid.len() {
         return Err(ClientError::MissingValue {
                 field: "Parameter count doesn't match between replica_uuid and snapshot_uuid".to_string()
             });
     }
+
     let replicas: Vec<v1_rpc::snapshot::NexusCreateSnapshotReplicaDescriptor> =
         replica_uuid
             .into_iter()

io-engine/tests/snapshot_nexus.rs

@@ -11,14 +11,22 @@ use common::{
             bdev::ListBdevOptions,
             pool::CreatePoolRequest,
             replica::{CreateReplicaRequest, ListReplicaOptions},
-            snapshot::{ListSnapshotsRequest, SnapshotInfo},
+            snapshot::{
+                ListSnapshotsRequest,
+                NexusCreateSnapshotReplicaDescriptor,
+                SnapshotInfo,
+            },
             GrpcConnect,
         },
+        Binary,
         Builder,
         ComposeTest,
         MayastorTest,
     },
+    nexus::NexusBuilder,
     nvme::{list_mayastor_nvme_devices, nvme_connect, nvme_disconnect_all},
+    pool::PoolBuilder,
+    replica::ReplicaBuilder,
 };
 
 use io_engine::{
@@ -319,47 +327,6 @@ async fn test_replica_handle_snapshot() {
     );
 }
 
-#[tokio::test]
-async fn test_multireplica_nexus_snapshot() {
-    let ms = get_ms();
-    let (_test, urls) = launch_instance(true).await;
-
-    ms.spawn(async move {
-        let nexus = create_nexus(&urls).await;
-
-        let snapshot_params = SnapshotParams::new(
-            Some(String::from("e1")),
-            Some(String::from("p1")),
-            Some(Uuid::new_v4().to_string()),
-            Some(String::from("s1")),
-            Some(Uuid::new_v4().to_string()),
-            Some(Utc::now().to_string()),
-            false,
-        );
-
-        let replicas = vec![
-            NexusReplicaSnapshotDescriptor {
-                replica_uuid: replica1_uuid(),
-                skip: false,
-                snapshot_uuid: Some(Uuid::new_v4().to_string()),
-            },
-            NexusReplicaSnapshotDescriptor {
-                replica_uuid: replica2_uuid(),
-                skip: false,
-                snapshot_uuid: Some(Uuid::new_v4().to_string()),
-            },
-        ];
-
-        nexus
-            .create_snapshot(snapshot_params, replicas)
-            .await
-            .expect_err(
-                "Snapshot successfully created for a multireplica nexus",
-            );
-    })
-    .await;
-}
-
 #[tokio::test]
 async fn test_list_no_snapshots() {
     let (test, _urls) = launch_instance(false).await;
@@ -1282,3 +1249,152 @@ async fn test_replica_listing_with_query() {
     assert!(!replicas[0].is_clone && !replicas[0].is_snapshot);
     assert!(!replicas[1].is_clone && !replicas[1].is_snapshot);
 }
+
+#[tokio::test]
+async fn test_multireplica_nexus_snapshot() {
+    const POOL_SIZE: u64 = 60;
+    const REPL_SIZE: u64 = 22;
+
+    common::composer_init();
+    let test = Builder::new()
+        .name("cargo-test")
+        .network("10.1.0.0/16")
+        .unwrap()
+        .add_container_bin(
+            "ms_nexus",
+            Binary::from_dbg("io-engine").with_args(vec!["-l", "1"]),
+        )
+        .add_container_bin(
+            "ms_repl_1",
+            Binary::from_dbg("io-engine").with_args(vec!["-l", "2"]),
+        )
+        .add_container_bin(
+            "ms_repl_2",
+            Binary::from_dbg("io-engine").with_args(vec!["-l", "3"]),
+        )
+        .add_container_bin(
+            "ms_repl_3",
+            Binary::from_dbg("io-engine").with_args(vec!["-l", "4"]),
+        )
+        .with_clean(true)
+        .build()
+        .await
+        .unwrap();
+
+    let conn = GrpcConnect::new(&test);
+    let ms_nexus = conn.grpc_handle_shared("ms_nexus").await.unwrap();
+    let ms_repl_1 = conn.grpc_handle_shared("ms_repl_1").await.unwrap();
+    let ms_repl_2 = conn.grpc_handle_shared("ms_repl_2").await.unwrap();
+    let ms_repl_3 = conn.grpc_handle_shared("ms_repl_3").await.unwrap();
+    // Create Pool-1 and Replica-1.
+    let mut pool_1 = PoolBuilder::new(ms_repl_1.clone())
+        .with_name("pool1")
+        .with_new_uuid()
+        .with_malloc("mem1", POOL_SIZE);
+
+    let mut repl_1 = ReplicaBuilder::new(ms_repl_1.clone())
+        .with_pool(&pool_1)
+        .with_name("r1")
+        .with_new_uuid()
+        .with_size_mb(REPL_SIZE)
+        .with_thin(true);
+
+    pool_1.create().await.unwrap();
+    repl_1.create().await.unwrap();
+    repl_1.share().await.unwrap();
+
+    // Create Pool-2 and Replica-2.
+    let mut pool_2 = PoolBuilder::new(ms_repl_2.clone())
+        .with_name("pool2")
+        .with_new_uuid()
+        .with_malloc("mem2", POOL_SIZE);
+
+    let mut repl_2 = ReplicaBuilder::new(ms_repl_2.clone())
+        .with_pool(&pool_2)
+        .with_name("r2")
+        .with_new_uuid()
+        .with_size_mb(REPL_SIZE)
+        .with_thin(true);
+
+    pool_2.create().await.unwrap();
+    repl_2.create().await.unwrap();
+    repl_2.share().await.unwrap();
+    // Create Pool-3 and Replica-3.
+    let mut pool_3 = PoolBuilder::new(ms_repl_3.clone())
+        .with_name("pool3")
+        .with_new_uuid()
+        .with_malloc("mem3", POOL_SIZE);
+
+    let mut repl_3 = ReplicaBuilder::new(ms_repl_3.clone())
+        .with_pool(&pool_3)
+        .with_name("r3")
+        .with_new_uuid()
+        .with_size_mb(REPL_SIZE)
+        .with_thin(true);
+
+    pool_3.create().await.unwrap();
+    repl_3.create().await.unwrap();
+    repl_3.share().await.unwrap();
+
+    // Create nexus.
+    let mut nex_0 = NexusBuilder::new(ms_nexus.clone())
+        .with_name("nexus0")
+        .with_new_uuid()
+        .with_size_mb(REPL_SIZE)
+        .with_replica(&repl_1)
+        .with_replica(&repl_2)
+        .with_replica(&repl_3);
+
+    nex_0.create().await.unwrap();
+    nex_0.publish().await.unwrap();
+
+    let snapshot_params = SnapshotParams::new(
+        Some(String::from("e1")),
+        Some(String::from("p1")),
+        Some(Uuid::new_v4().to_string()),
+        Some(String::from("s1")),
+        Some(Uuid::new_v4().to_string()),
+        Some(Utc::now().to_string()),
+        false,
+    );
+
+    let mut replicas = vec![
+        NexusCreateSnapshotReplicaDescriptor {
+            replica_uuid: repl_1.uuid(),
+            snapshot_uuid: Some(Uuid::new_v4().to_string()),
+            skip: false,
+        },
+        NexusCreateSnapshotReplicaDescriptor {
+            replica_uuid: repl_2.uuid(),
+            snapshot_uuid: Some(Uuid::new_v4().to_string()),
+            skip: false,
+        },
+    ];
+    // check for error when snapshot uuid is not provided for all replicas.
+    let result = nex_0
+        .create_nexus_snapshot(&snapshot_params, &replicas)
+        .await;
+    assert!(result.is_err());
+
+    replicas.push(NexusCreateSnapshotReplicaDescriptor {
+        replica_uuid: repl_2.uuid(),
+        snapshot_uuid: replicas[1].snapshot_uuid.clone(),
+        skip: false,
+    });
+    // check for error when snapshot uuid is duplicated.
+    let result = nex_0
+        .create_nexus_snapshot(&snapshot_params, &replicas)
+        .await;
+    assert!(result.is_err());
+    replicas.pop();
+    replicas.push(NexusCreateSnapshotReplicaDescriptor {
+        replica_uuid: repl_3.uuid(),
+        snapshot_uuid: Some(Uuid::new_v4().to_string()),
+        skip: false,
+    });
+    let snap_list = nex_0
+        .create_nexus_snapshot(&snapshot_params, &replicas)
+        .await
+        .unwrap();
+    assert_eq!(snap_list.len(), 3);
+}