Skip to content

Commit

Permalink
[thin_dump] Prompt for repair on input errors
Browse files Browse the repository at this point in the history
thin_dump should display the repairing hint if there's any error in the
input metadata rather than the output process. A context object thus is
added to the returned error for callers to identify the error type.

Note that a broken pipe error (EPIPE) is returned as an output error since
the Rust std library ignores SIGPIPE by default [1][2]. However, due to
the dump process leverages NodeVisitor, a low-level output error is
stringified into a BTreeError, and thus the caller cannot tell whether
it's broken pipe or not.

[1] rust-lang/rust#13158
[2] rust-lang/rust#62569
  • Loading branch information
mingnus committed May 27, 2022
1 parent 69ff4d4 commit c0d72ec
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 38 deletions.
17 changes: 13 additions & 4 deletions src/commands/thin_dump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::sync::Arc;

use crate::commands::utils::*;
use crate::commands::Command;
use crate::dump_utils::OutputError;
use crate::report::*;
use crate::thin::dump::{dump, ThinDumpOptions};
use crate::thin::metadata_repair::SuperblockOverrides;
Expand Down Expand Up @@ -128,9 +129,17 @@ impl<'a> Command<'a> for ThinDumpCommand {
},
};

dump(opts).map_err(|reason| {
report.fatal(&format!("{}", reason));
std::io::Error::from_raw_os_error(libc::EPERM)
})
if let Err(e) = dump(opts) {
if !e.is::<OutputError>() {
report.fatal(&format!("{:?}", e));
report.fatal(
"metadata contains errors (run thin_check for details).\n\
perhaps you wanted to run with --repair ?",
);
}
return Err(std::io::Error::from_raw_os_error(libc::EPERM));
}

Ok(())
}
}
13 changes: 13 additions & 0 deletions src/dump_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//------------------------------------------

// A wrapper for callers to identify the error type
#[derive(Debug)]
pub struct OutputError;

impl std::fmt::Display for OutputError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "output error")
}
}

//------------------------------------------
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod cache;
pub mod checksum;
pub mod commands;
pub mod copier;
pub mod dump_utils;
pub mod era;
pub mod file_utils;
pub mod grid_layout;
Expand Down
71 changes: 37 additions & 34 deletions src/thin/dump.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use anyhow::{anyhow, Result};
use anyhow::{anyhow, Context, Result};
use std::fs::File;
use std::io::BufWriter;
use std::io::Write;
use std::path::Path;
use std::sync::{Arc, Mutex};

use crate::checksum;
use crate::dump_utils::*;
use crate::io_engine::{AsyncIoEngine, Block, IoEngine, SyncIoEngine};
use crate::pdata::btree::{self, *};
use crate::pdata::btree_walker::*;
Expand Down Expand Up @@ -108,6 +109,9 @@ impl<'a> NodeVisitor<BlockTime> for MappingVisitor<'a> {
let mut inner = self.inner.lock().unwrap();
for (k, v) in keys.iter().zip(values.iter()) {
if let Some(run) = inner.builder.next(*k, v.block, v.time) {
// FIXME: BTreeError should carry more information than a string
// so the caller could identify the actual root cause,
// e.g., a broken pipe error or something.
inner
.md_out
.map(&run)
Expand Down Expand Up @@ -153,58 +157,49 @@ pub struct ThinDumpOptions<'a> {
pub overrides: SuperblockOverrides,
}

struct Context {
struct ThinDumpContext {
report: Arc<Report>,
engine: Arc<dyn IoEngine + Send + Sync>,
}

fn mk_context(opts: &ThinDumpOptions) -> Result<Context> {
fn mk_context(opts: &ThinDumpOptions) -> Result<ThinDumpContext> {
let engine: Arc<dyn IoEngine + Send + Sync> = if opts.async_io {
Arc::new(AsyncIoEngine::new(opts.input, MAX_CONCURRENT_IO, false)?)
} else {
let nr_threads = std::cmp::max(8, num_cpus::get() * 2);
Arc::new(SyncIoEngine::new(opts.input, nr_threads, false)?)
};

Ok(Context {
Ok(ThinDumpContext {
report: opts.report.clone(),
engine,
})
}

//------------------------------------------

fn emit_leaf(v: &mut MappingVisitor, b: &Block) -> Result<()> {
fn emit_leaf(v: &MappingVisitor, b: &Block) -> Result<()> {
use Node::*;
let path = Vec::new();
let kr = KeyRange::new();

let bt = checksum::metadata_block_type(b.get_data());
if bt != checksum::BT::NODE {
return Err(anyhow!(format!(
"checksum failed for node {}, {:?}",
b.loc, bt
)));
return Err(anyhow!("checksum failed for node {}, {:?}", b.loc, bt));
}

let node = unpack_node::<BlockTime>(&path, b.get_data(), true, true)?;

match node {
Internal { .. } => {
return Err(anyhow!("not a leaf"));
}
Internal { .. } => Err(anyhow!("block {} is not a leaf", b.loc)),
Leaf {
header,
keys,
values,
} => {
if let Err(_e) = v.visit(&path, &kr, &header, &keys, &values) {
return Err(anyhow!("couldn't emit leaf node"));
}
}
} => v
.visit(&path, &kr, &header, &keys, &values)
.context(OutputError),
}

Ok(())
}

fn read_for<T>(engine: Arc<dyn IoEngine>, blocks: &[u64], mut t: T) -> Result<()>
Expand All @@ -216,22 +211,27 @@ where
.read_many(cs)
.map_err(|_e| anyhow!("read_many failed"))?
{
t(b.map_err(|_e| anyhow!("read of individual block failed"))?)?;
let blk = b.map_err(|_e| anyhow!("read of individual block failed"))?;
t(blk)?;
}
}

Ok(())
}

fn emit_leaves(engine: Arc<dyn IoEngine>, out: &mut dyn MetadataVisitor, ls: &[u64]) -> Result<()> {
let mut v = MappingVisitor::new(out);
fn emit_leaves(
engine: Arc<dyn IoEngine>,
out: &mut dyn MetadataVisitor,
leaves: &[u64],
) -> Result<()> {
let v = MappingVisitor::new(out);
let proc = |b| {
emit_leaf(&mut v, &b)?;
emit_leaf(&v, &b)?;
Ok(())
};

read_for(engine, ls, proc)?;
v.end_walk().map_err(|_| anyhow!("failed to emit leaves"))
read_for(engine, leaves, proc)?;
v.end_walk().context(OutputError)
}

fn emit_entries(
Expand All @@ -252,7 +252,7 @@ fn emit_entries(
leaves.clear();
}
let str = format!("{}", id);
out.ref_shared(&str)?;
out.ref_shared(&str).context(OutputError)?;
}
}
}
Expand Down Expand Up @@ -281,12 +281,13 @@ pub fn dump_metadata(
nr_data_blocks: data_root.nr_blocks,
metadata_snap: None,
};
out.superblock_b(&out_sb)?;
out.superblock_b(&out_sb).context(OutputError)?;

for d in &md.defs {
out.def_shared_b(&format!("{}", d.def_id))?;
out.def_shared_b(&format!("{}", d.def_id))
.context(OutputError)?;
emit_entries(engine.clone(), out, &d.map.entries)?;
out.def_shared_e()?;
out.def_shared_e().context(OutputError)?;
}

for dev in &md.devs {
Expand All @@ -297,12 +298,12 @@ pub fn dump_metadata(
creation_time: dev.detail.creation_time,
snap_time: dev.detail.snapshotted_time,
};
out.device_b(&device)?;
out.device_b(&device).context(OutputError)?;
emit_entries(engine.clone(), out, &dev.map.entries)?;
out.device_e()?;
out.device_e().context(OutputError)?;
}
out.superblock_e()?;
out.eof()?;
out.superblock_e().context(OutputError)?;
out.eof().context(OutputError)?;

Ok(())
}
Expand All @@ -324,11 +325,13 @@ pub fn dump(opts: ThinDumpOptions) -> Result<()> {
read_superblock(ctx.engine.as_ref(), SUPERBLOCK_LOCATION)
.and_then(|sb| sb.overrides(&opts.overrides))?
};

let md = build_metadata(ctx.engine.clone(), &sb)?;
let md = optimise_metadata(md)?;

let writer: Box<dyn Write> = if opts.output.is_some() {
Box::new(BufWriter::new(File::create(opts.output.unwrap())?))
let f = File::create(opts.output.unwrap()).context(OutputError)?;
Box::new(BufWriter::new(f))
} else {
Box::new(BufWriter::new(std::io::stdout()))
};
Expand Down

0 comments on commit c0d72ec

Please sign in to comment.