improve error when db and dir meta don't match

I saw this recently while working on new-schema. It was probably due
to some manual upgrade or downgrade I did rather than an actual bug.
Improve debuggability a little nonetheless.
This commit is contained in:
Scott Lamb 2021-09-22 12:39:02 -07:00
parent 901ba121a2
commit 66f76079c0
3 changed files with 44 additions and 40 deletions

View File

@ -334,7 +334,7 @@ impl SampleFileDir {
} }
/// Returns expected existing metadata when opening this directory. /// Returns expected existing metadata when opening this directory.
fn meta(&self, db_uuid: &Uuid) -> schema::DirMeta { fn expected_meta(&self, db_uuid: &Uuid) -> schema::DirMeta {
let mut meta = schema::DirMeta::default(); let mut meta = schema::DirMeta::default();
meta.db_uuid.extend_from_slice(&db_uuid.as_bytes()[..]); meta.db_uuid.extend_from_slice(&db_uuid.as_bytes()[..]);
meta.dir_uuid.extend_from_slice(&self.uuid.as_bytes()[..]); meta.dir_uuid.extend_from_slice(&self.uuid.as_bytes()[..]);
@ -1182,20 +1182,20 @@ impl LockedDatabase {
if dir.dir.is_some() { if dir.dir.is_some() {
continue; continue;
} }
let mut meta = dir.meta(&self.uuid); let mut expected_meta = dir.expected_meta(&self.uuid);
if let Some(o) = self.open.as_ref() { if let Some(o) = self.open.as_ref() {
let open = meta.in_progress_open.set_default(); let open = expected_meta.in_progress_open.set_default();
open.id = o.id; open.id = o.id;
open.uuid.extend_from_slice(&o.uuid.as_bytes()[..]); open.uuid.extend_from_slice(&o.uuid.as_bytes()[..]);
} }
let d = dir::SampleFileDir::open(&dir.path, &meta) let d = dir::SampleFileDir::open(&dir.path, &expected_meta)
.map_err(|e| e.context(format!("Failed to open dir {}", dir.path)))?; .map_err(|e| e.context(format!("Failed to open dir {}", dir.path)))?;
if self.open.is_none() { if self.open.is_none() {
// read-only mode; it's already fully opened. // read-only mode; it's already fully opened.
dir.dir = Some(d); dir.dir = Some(d);
} else { } else {
// read-write mode; there are more steps to do. // read-write mode; there are more steps to do.
e.insert((meta, d)); e.insert((expected_meta, d));
} }
} }
@ -1221,8 +1221,7 @@ impl LockedDatabase {
for (id, (mut meta, d)) in in_progress.drain() { for (id, (mut meta, d)) in in_progress.drain() {
let dir = self.sample_file_dirs_by_id.get_mut(&id).unwrap(); let dir = self.sample_file_dirs_by_id.get_mut(&id).unwrap();
meta.last_complete_open.clear(); meta.last_complete_open = meta.in_progress_open.take().into();
mem::swap(&mut meta.last_complete_open, &mut meta.in_progress_open);
d.write_meta(&meta)?; d.write_meta(&meta)?;
dir.dir = Some(d); dir.dir = Some(d);
} }
@ -1786,14 +1785,13 @@ impl LockedDatabase {
path, path,
uuid, uuid,
dir: Some(dir), dir: Some(dir),
last_complete_open: None, last_complete_open: Some(*o),
garbage_needs_unlink: FnvHashSet::default(), garbage_needs_unlink: FnvHashSet::default(),
garbage_unlinked: Vec::new(), garbage_unlinked: Vec::new(),
}), }),
Entry::Occupied(_) => bail!("duplicate sample file dir id {}", id), Entry::Occupied(_) => bail!("duplicate sample file dir id {}", id),
}; };
d.last_complete_open = Some(*o); meta.last_complete_open = meta.in_progress_open.take().into();
mem::swap(&mut meta.last_complete_open, &mut meta.in_progress_open);
d.dir.as_ref().unwrap().write_meta(&meta)?; d.dir.as_ref().unwrap().write_meta(&meta)?;
Ok(id) Ok(id)
} }
@ -1815,7 +1813,7 @@ impl LockedDatabase {
); );
} }
let dir = match d.get_mut().dir.take() { let dir = match d.get_mut().dir.take() {
None => dir::SampleFileDir::open(&d.get().path, &d.get().meta(&self.uuid))?, None => dir::SampleFileDir::open(&d.get().path, &d.get().expected_meta(&self.uuid))?,
Some(arc) => match Arc::strong_count(&arc) { Some(arc) => match Arc::strong_count(&arc) {
1 => { 1 => {
d.get_mut().dir = Some(arc); // put it back. d.get_mut().dir = Some(arc); // put it back.
@ -1830,7 +1828,7 @@ impl LockedDatabase {
&d.get().path &d.get().path
); );
} }
let mut meta = d.get().meta(&self.uuid); let mut meta = d.get().expected_meta(&self.uuid);
meta.in_progress_open = meta.last_complete_open.take().into(); meta.in_progress_open = meta.last_complete_open.take().into();
dir.write_meta(&meta)?; dir.write_meta(&meta)?;
if self if self

View File

@ -212,8 +212,8 @@ impl SampleFileDir {
/// ///
/// `db_meta.in_progress_open` should be filled if the directory should be opened in read/write /// `db_meta.in_progress_open` should be filled if the directory should be opened in read/write
/// mode; absent in read-only mode. /// mode; absent in read-only mode.
pub fn open(path: &str, db_meta: &schema::DirMeta) -> Result<Arc<SampleFileDir>, Error> { pub fn open(path: &str, expected_meta: &schema::DirMeta) -> Result<Arc<SampleFileDir>, Error> {
let read_write = db_meta.in_progress_open.is_some(); let read_write = expected_meta.in_progress_open.is_some();
let s = SampleFileDir::open_self(path, false)?; let s = SampleFileDir::open_self(path, false)?;
s.fd.lock(if read_write { s.fd.lock(if read_write {
FlockArg::LockExclusiveNonblock FlockArg::LockExclusiveNonblock
@ -222,45 +222,50 @@ impl SampleFileDir {
}) })
.map_err(|e| e.context(format!("unable to lock dir {}", path)))?; .map_err(|e| e.context(format!("unable to lock dir {}", path)))?;
let dir_meta = read_meta(&s.fd).map_err(|e| e.context("unable to read meta file"))?; let dir_meta = read_meta(&s.fd).map_err(|e| e.context("unable to read meta file"))?;
if !SampleFileDir::consistent(db_meta, &dir_meta) { if let Err(e) = SampleFileDir::check_consistent(expected_meta, &dir_meta) {
let serialized = db_meta
.write_length_delimited_to_bytes()
.expect("proto3->vec is infallible");
bail!( bail!(
"metadata mismatch.\ndb: {:#?}\ndir: {:#?}\nserialized db: {:#?}", "metadata mismatch: {}.\nexpected:\n{:#?}\n\nactual:\n{:#?}",
db_meta, e,
&dir_meta, expected_meta,
&serialized &dir_meta
); );
} }
if db_meta.in_progress_open.is_some() { if expected_meta.in_progress_open.is_some() {
s.write_meta(db_meta)?; s.write_meta(expected_meta)?;
} }
Ok(s) Ok(s)
} }
/// Returns true if the existing directory and database metadata are consistent; the directory /// Checks that the existing directory and database metadata are consistent; the directory
/// is then openable. /// is then openable.
pub(crate) fn consistent(db_meta: &schema::DirMeta, dir_meta: &schema::DirMeta) -> bool { pub(crate) fn check_consistent(
if dir_meta.db_uuid != db_meta.db_uuid { expected_meta: &schema::DirMeta,
return false; actual_meta: &schema::DirMeta,
) -> Result<(), String> {
if actual_meta.db_uuid != expected_meta.db_uuid {
return Err("db uuid mismatch".into());
} }
if dir_meta.dir_uuid != db_meta.dir_uuid { if actual_meta.dir_uuid != expected_meta.dir_uuid {
return false; return Err("dir uuid mismatch".into());
} }
if db_meta.last_complete_open.is_some() if expected_meta.last_complete_open.is_some()
&& (db_meta.last_complete_open != dir_meta.last_complete_open && (expected_meta.last_complete_open != actual_meta.last_complete_open
&& db_meta.last_complete_open != dir_meta.in_progress_open) && expected_meta.last_complete_open != actual_meta.in_progress_open)
{ {
return false; return Err(format!(
"expected open {:?}; but got {:?} (complete) or {:?} (in progress)",
&expected_meta.last_complete_open,
&actual_meta.last_complete_open,
&actual_meta.in_progress_open,
));
} }
if db_meta.last_complete_open.is_none() && dir_meta.last_complete_open.is_some() { if expected_meta.last_complete_open.is_none() && actual_meta.last_complete_open.is_some() {
return false; return Err("expected never opened".into());
} }
true Ok(())
} }
pub(crate) fn create( pub(crate) fn create(

View File

@ -37,11 +37,12 @@ fn maybe_upgrade_meta(dir: &dir::Fd, db_meta: &schema::DirMeta) -> Result<bool,
dir_meta dir_meta
.merge_from(&mut s) .merge_from(&mut s)
.map_err(|e| e.context("Unable to parse metadata proto: {}"))?; .map_err(|e| e.context("Unable to parse metadata proto: {}"))?;
if !dir::SampleFileDir::consistent(&db_meta, &dir_meta) { if let Err(e) = dir::SampleFileDir::check_consistent(&db_meta, &dir_meta) {
bail!( bail!(
"Inconsistent db_meta={:?} dir_meta={:?}", "Inconsistent db_meta={:?} dir_meta={:?}: {}",
&db_meta, &db_meta,
&dir_meta &dir_meta,
e
); );
} }
let mut f = crate::fs::openat( let mut f = crate::fs::openat(