From e704728ee72f6a5a3ebf6e8bfeee08cf58d44153 Mon Sep 17 00:00:00 2001 From: Tamo Date: Mon, 6 Mar 2023 13:30:06 +0100 Subject: [PATCH] fix the snapshots permissions on unix system --- index-scheduler/src/batch.rs | 11 ++++++++--- meilisearch/tests/snapshot/mod.rs | 22 +++++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/index-scheduler/src/batch.rs b/index-scheduler/src/batch.rs index 66c516d9b..5bb75bcba 100644 --- a/index-scheduler/src/batch.rs +++ b/index-scheduler/src/batch.rs @@ -675,9 +675,6 @@ impl IndexScheduler { } // 3. Snapshot every indexes - // TODO we are opening all of the indexes it can be too much we should unload all - // of the indexes we are trying to open. It would be even better to only unload - // the ones that were opened by us. Or maybe use a LRU in the index mapper. for result in self.index_mapper.index_mapping.iter(&rtxn)? { let (name, uuid) = result?; let index = self.index_mapper.index(&rtxn, name)?; @@ -714,6 +711,14 @@ impl IndexScheduler { // 5.3 Change the permission to make the snapshot readonly let mut permissions = file.metadata()?.permissions(); permissions.set_readonly(true); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + #[allow(clippy::non_octal_unix_permissions)] + // rwxrwxrwx + permissions.set_mode(0b100100100); + } + file.set_permissions(permissions)?; for task in &mut tasks { diff --git a/meilisearch/tests/snapshot/mod.rs b/meilisearch/tests/snapshot/mod.rs index 4615f3ea9..fa2b1a272 100644 --- a/meilisearch/tests/snapshot/mod.rs +++ b/meilisearch/tests/snapshot/mod.rs @@ -1,8 +1,8 @@ use std::time::Duration; +use actix_rt::time::sleep; use meilisearch::option::ScheduleSnapshot; use meilisearch::Opt; -use tokio::time::sleep; use crate::common::server::default_settings; use crate::common::{GetAllDocumentsOptions, Server}; @@ -23,14 +23,13 @@ macro_rules! verify_snapshot { }; let (snapshot, _) = test(snapshot.clone()).await; let (orig, _) = test(orig.clone()).await; - assert_eq!(snapshot, orig); + assert_eq!(snapshot, orig, "Got \n{}\nWhile expecting:\n{}", serde_json::to_string_pretty(&snapshot).unwrap(), serde_json::to_string_pretty(&orig).unwrap()); } )* }; } #[actix_rt::test] -#[ignore] // TODO: unignore async fn perform_snapshot() { let temp = tempfile::tempdir().unwrap(); let snapshot_dir = tempfile::tempdir().unwrap(); @@ -56,11 +55,21 @@ async fn perform_snapshot() { index.wait_task(2).await; - sleep(Duration::from_secs(2)).await; + sleep(Duration::from_secs(1)).await; let temp = tempfile::tempdir().unwrap(); let snapshot_path = snapshot_dir.path().to_owned().join("db.snapshot"); + #[cfg_attr(windows, allow(unused))] + let snapshot_meta = std::fs::metadata(&snapshot_path).unwrap(); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = snapshot_meta.permissions().mode(); + // rwxrwxrwx + meili_snap::snapshot!(format!("{:b}", mode), @"1000000100100100"); + } let options = Opt { import_snapshot: Some(snapshot_path), ..default_settings(temp.path()) }; @@ -71,7 +80,10 @@ async fn perform_snapshot() { // for some reason the db sizes differ. this may be due to the compaction options we have // set when performing the snapshot //server.stats(), - server.tasks(), + + // We can't test all the tasks contained in the snapshot because the on the original instance the snapshotCreation task was added + server.tasks_filter("?from=4"), + server.index("test").get_all_documents(GetAllDocumentsOptions::default()), server.index("test").settings(), server.index("test1").get_all_documents(GetAllDocumentsOptions::default()),