3561: Fix the snapshots permissions on unix system r=irevoire a=irevoire

# Pull Request

## Related issue
Fixes https://github.com/meilisearch/meilisearch/issues/3507

The snapshot permissions were wrong after the v0.30 and the huge refacto of the index scheduler.
Fix this issue + add a test on the permissions on unix

Co-authored-by: Tamo <tamo@meilisearch.com>
This commit is contained in:
bors[bot] 2023-03-07 08:51:38 +00:00 committed by GitHub
commit 370d88f626
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 8 deletions

View File

@ -675,9 +675,6 @@ impl IndexScheduler {
} }
// 3. Snapshot every indexes // 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)? { for result in self.index_mapper.index_mapping.iter(&rtxn)? {
let (name, uuid) = result?; let (name, uuid) = result?;
let index = self.index_mapper.index(&rtxn, name)?; let index = self.index_mapper.index(&rtxn, name)?;
@ -714,6 +711,14 @@ impl IndexScheduler {
// 5.3 Change the permission to make the snapshot readonly // 5.3 Change the permission to make the snapshot readonly
let mut permissions = file.metadata()?.permissions(); let mut permissions = file.metadata()?.permissions();
permissions.set_readonly(true); 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)?; file.set_permissions(permissions)?;
for task in &mut tasks { for task in &mut tasks {

View File

@ -1,8 +1,8 @@
use std::time::Duration; use std::time::Duration;
use actix_rt::time::sleep;
use meilisearch::option::ScheduleSnapshot; use meilisearch::option::ScheduleSnapshot;
use meilisearch::Opt; use meilisearch::Opt;
use tokio::time::sleep;
use crate::common::server::default_settings; use crate::common::server::default_settings;
use crate::common::{GetAllDocumentsOptions, Server}; use crate::common::{GetAllDocumentsOptions, Server};
@ -23,21 +23,20 @@ macro_rules! verify_snapshot {
}; };
let (snapshot, _) = test(snapshot.clone()).await; let (snapshot, _) = test(snapshot.clone()).await;
let (orig, _) = test(orig.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] #[actix_rt::test]
#[ignore] // TODO: unignore
async fn perform_snapshot() { async fn perform_snapshot() {
let temp = tempfile::tempdir().unwrap(); let temp = tempfile::tempdir().unwrap();
let snapshot_dir = tempfile::tempdir().unwrap(); let snapshot_dir = tempfile::tempdir().unwrap();
let options = Opt { let options = Opt {
snapshot_dir: snapshot_dir.path().to_owned(), snapshot_dir: snapshot_dir.path().to_owned(),
schedule_snapshot: ScheduleSnapshot::Enabled(1), schedule_snapshot: ScheduleSnapshot::Enabled(2),
..default_settings(temp.path()) ..default_settings(temp.path())
}; };
@ -61,6 +60,16 @@ async fn perform_snapshot() {
let temp = tempfile::tempdir().unwrap(); let temp = tempfile::tempdir().unwrap();
let snapshot_path = snapshot_dir.path().to_owned().join("db.snapshot"); 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()) }; 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 // for some reason the db sizes differ. this may be due to the compaction options we have
// set when performing the snapshot // set when performing the snapshot
//server.stats(), //server.stats(),
server.tasks(),
// The original instance contains the snapshotCreation task, while the snapshotted-instance does not. For this reason we need to compare the task queue **after** the task 4
server.tasks_filter("?from=2"),
server.index("test").get_all_documents(GetAllDocumentsOptions::default()), server.index("test").get_all_documents(GetAllDocumentsOptions::default()),
server.index("test").settings(), server.index("test").settings(),
server.index("test1").get_all_documents(GetAllDocumentsOptions::default()), server.index("test1").get_all_documents(GetAllDocumentsOptions::default()),