Skip to content

Commit

Permalink
fix: Accidental full manifest for some files
Browse files Browse the repository at this point in the history
  • Loading branch information
schneiderstefan committed Jan 10, 2024
1 parent fb2cf77 commit 794a7ba
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 6 deletions.
2 changes: 1 addition & 1 deletion rs/state_manager/src/manifest.rs
Expand Up @@ -830,7 +830,7 @@ fn dirty_pages_to_dirty_chunks(
let new_path = checkpoint.raw_path().join(path);
let old_path = manifest_delta.base_checkpoint.raw_path().join(path);
if !old_path.exists() {
break;
continue;
}
let new_metadata = new_path.metadata();
let old_metadata = old_path.metadata();
Expand Down
78 changes: 73 additions & 5 deletions rs/state_manager/src/manifest/tests/computation.rs
@@ -1,25 +1,31 @@
use crate::manifest::{
build_file_group_chunks, build_meta_manifest, compute_manifest, diff_manifest,
file_chunk_range, filter_out_zero_chunks, hash::ManifestHash, manifest_hash, manifest_hash_v1,
manifest_hash_v2, meta_manifest_hash, validate_chunk, validate_manifest,
validate_manifest_internal_consistency, validate_meta_manifest, validate_sub_manifest,
ChunkValidationError, DiffScript, ManifestMetrics, ManifestValidationError, StateSyncVersion,
DEFAULT_CHUNK_SIZE, MAX_FILE_SIZE_TO_GROUP,
dirty_pages_to_dirty_chunks, file_chunk_range, files_with_sizes, filter_out_zero_chunks,
hash::ManifestHash, manifest_hash, manifest_hash_v1, manifest_hash_v2, meta_manifest_hash,
validate_chunk, validate_manifest, validate_manifest_internal_consistency,
validate_meta_manifest, validate_sub_manifest, ChunkValidationError, DiffScript, ManifestDelta,
ManifestMetrics, ManifestValidationError, StateSyncVersion, DEFAULT_CHUNK_SIZE,
MAX_FILE_SIZE_TO_GROUP,
};
use crate::state_sync::types::{
decode_manifest, encode_manifest, ChunkInfo, FileGroupChunks, FileInfo, Manifest, MetaManifest,
FILE_GROUP_CHUNK_ID_OFFSET,
};
use crate::DirtyPages;

use bit_vec::BitVec;
use ic_config::flag_status::FlagStatus;
use ic_crypto_sha2::Sha256;
use ic_logger::replica_logger::no_op_logger;
use ic_metrics::MetricsRegistry;
use ic_state_layout::{CheckpointLayout, CANISTER_FILE};
use ic_test_utilities_tmpdir::tmpdir;
use ic_types::state_sync::CURRENT_STATE_SYNC_VERSION;
use ic_types::{crypto::CryptoHash, CryptoHashOfState, Height};
use maplit::btreemap;
use std::collections::{HashMap, HashSet};
use std::fs;
use std::io::Write;
use std::path::PathBuf;
use strum::IntoEnumIterator;

Expand Down Expand Up @@ -1292,3 +1298,65 @@ fn test_file_index_independent_file_hash() {
assert_eq!(file1_hash_before, file1_hash_after);
assert_eq!(file3_hash_before, file3_hash_after);
}

#[test]
fn all_same_inodes_are_detected() {
use std::fs::hard_link;
use std::fs::File;

let base = tmpdir("base");
let target = tmpdir("target");

let create_file_in_target = |name| {
let mut file = File::create(target.path().join(name)).unwrap();
file.write_all(b"data").unwrap();
};
let create_different_file_in_base_and_target = |name| {
let mut file = File::create(base.path().join(name)).unwrap();
file.write_all(b"data").unwrap();
let mut file = File::create(target.path().join(name)).unwrap();
file.write_all(b"data").unwrap();
};
let create_same_file_in_base_and_target = |name| {
let mut file = File::create(base.path().join(name)).unwrap();
file.write_all(b"data").unwrap();
hard_link(base.path().join(name), target.path().join(name)).unwrap();
};

create_file_in_target("a_new");
create_different_file_in_base_and_target("b_changed");
create_same_file_in_base_and_target("c_same");
create_different_file_in_base_and_target("d_changed");
create_same_file_in_base_and_target("e_same");

let manifest_delta = ManifestDelta {
base_manifest: Manifest::new(StateSyncVersion::V0, vec![], vec![]),
base_height: Height::new(0),
target_height: Height::new(1),
dirty_memory_pages: DirtyPages::default(),
base_checkpoint: CheckpointLayout::new_untracked(base.path().to_path_buf(), Height::new(0))
.unwrap(),
lsmt_storage: FlagStatus::Enabled,
};

let mut files = Vec::new();
files_with_sizes(target.path(), "".into(), &mut files).unwrap();

let result = dirty_pages_to_dirty_chunks(
&no_op_logger(),
&manifest_delta,
&CheckpointLayout::new_untracked(target.path().to_path_buf(), Height::new(1)).unwrap(),
&files,
1024 * 1024,
)
.unwrap();

// All files are shorter than a chunk
let chunks = 1;
let expected = btreemap! {
PathBuf::from("c_same") => BitVec::from_elem(chunks, false),
PathBuf::from("e_same") => BitVec::from_elem(chunks, false),
};

assert_eq!(result, expected);
}

0 comments on commit 794a7ba

Please sign in to comment.