Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A builder for the Downstairs Downstairs struct. #1152

Merged
merged 4 commits into from
Mar 1, 2024
Merged

A builder for the Downstairs Downstairs struct. #1152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
06f466f
A builder for the downstairs Downstairs struct
leftwo Feb 8, 2024
c49876c
whitespace removal
leftwo Feb 8, 2024
14636b2
Merge branch 'main' into alan/downstairs-builder
leftwo Mar 1, 2024
d5ba912
Update after merge
leftwo Mar 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions downstairs/src/admin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,17 @@ pub async fn run_downstairs_for_region(
));
}

let d = build_downstairs_for_region(
&run_params.data,
run_params.lossy,
run_params.read_errors,
run_params.write_errors,
run_params.flush_errors,
run_params.read_only,
None,
)
.await
.map_err(|e| HttpError::for_internal_error(e.to_string()))?;
let mut d = Downstairs::new_builder(&run_params.data, run_params.read_only);
let d = d
.set_lossy(run_params.lossy)
.set_test_errors(
run_params.read_errors,
run_params.write_errors,
run_params.flush_errors,
)
.build()
.await
.map_err(|e| HttpError::for_internal_error(e.to_string()))?;

let _join_handle = start_downstairs(
d.clone(),
Expand Down
285 changes: 126 additions & 159 deletions downstairs/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,99 @@ pub struct ActiveUpstairs {
pub terminate_sender: oneshot::Sender<UpstairsConnection>,
}

#[derive(Debug)]
pub struct DownstairsBuilder<'a> {
data: &'a Path,
read_only: bool,
lossy: Option<bool>,
read_errors: Option<bool>, // Test flag
write_errors: Option<bool>, // Test flag
flush_errors: Option<bool>, // Test flag
backend: Option<Backend>,
log: Option<Logger>,
}

impl DownstairsBuilder<'_> {
pub fn set_lossy(&mut self, lossy: bool) -> &mut Self {
self.lossy = Some(lossy);
self
}
pub fn set_test_errors(
&mut self,
read_errors: bool,
write_errors: bool,
flush_errors: bool,
) -> &mut Self {
self.read_errors = Some(read_errors);
self.write_errors = Some(write_errors);
self.flush_errors = Some(flush_errors);
self
}
pub fn set_backend(&mut self, backend: Backend) -> &mut Self {
self.backend = Some(backend);
self
}
pub fn set_logger(&mut self, log: Logger) -> &mut Self {
self.log = Some(log);
self
}

pub async fn build(&mut self) -> Result<Arc<Mutex<Downstairs>>> {
let lossy = self.lossy.unwrap_or(false);
let read_errors = self.read_errors.unwrap_or(false);
let write_errors = self.write_errors.unwrap_or(false);
let flush_errors = self.flush_errors.unwrap_or(false);
let backend = self.backend.unwrap_or(Backend::RawFile);

let log = match &self.log {
Some(log) => log.clone(),
None => build_logger(),
};

// Open the region at the provided location.
let region = Region::open_with_backend(
self.data,
Default::default(),
true,
self.read_only,
backend,
&log,
)
.await?;

let encrypted = region.encrypted();

let dss = DsStatOuter {
ds_stat_wrap: Arc::new(std::sync::Mutex::new(DsCountStat::new(
region.def().uuid(),
))),
};

info!(log, "UUID: {:?}", region.def().uuid());
info!(
log,
"Blocks per extent:{} Total Extents: {}",
region.def().extent_size().value,
region.def().extent_count(),
);

Ok(Arc::new(Mutex::new(Downstairs {
region,
lossy,
read_errors,
write_errors,
flush_errors,
active_upstairs: HashMap::new(),
dss,
read_only: self.read_only,
encrypted,
address: None,
repair_address: None,
log,
})))
}
}

/*
* Overall structure for things the downstairs is tracking.
* This includes the extents and their status as well as the
Expand All @@ -1374,34 +1467,16 @@ pub struct Downstairs {

#[allow(clippy::too_many_arguments)]
impl Downstairs {
fn new(
region: Region,
lossy: bool,
read_errors: bool,
write_errors: bool,
flush_errors: bool,
read_only: bool,
encrypted: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do we configure whether or not encryption is used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess you can't any more when starting a downstairs.

The region creation itself is where the decision to have or not have encryption is made. When we start a downstairs, the opening of the region will auto-select the encryption state.

If we allow start of a downstairs to pick what encryption it has, all that will end up doing is:
1: Match what the region open finds and move forward.
2: Not match the region open, and exit with error or panic.

Do we need to have this ability in the downstairs run path?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely not, ignore my comment!

log: Logger,
) -> Self {
let dss = DsStatOuter {
ds_stat_wrap: Arc::new(std::sync::Mutex::new(DsCountStat::new(
region.def().uuid(),
))),
};
Downstairs {
region,
lossy,
read_errors,
write_errors,
flush_errors,
active_upstairs: HashMap::new(),
dss,
pub fn new_builder(data: &Path, read_only: bool) -> DownstairsBuilder {
DownstairsBuilder {
data,
read_only,
encrypted,
address: None,
repair_address: None,
log,
lossy: Some(false),
read_errors: Some(false),
write_errors: Some(false),
flush_errors: Some(false),
backend: Some(Backend::RawFile),
log: None,
}
}

Expand Down Expand Up @@ -3069,78 +3144,6 @@ pub async fn create_region_with_backend(
Ok(region)
}

pub async fn build_downstairs_for_region(
data: &Path,
lossy: bool,
read_errors: bool,
write_errors: bool,
flush_errors: bool,
read_only: bool,
log_request: Option<Logger>,
) -> Result<Arc<Mutex<Downstairs>>> {
build_downstairs_for_region_with_backend(
data,
lossy,
read_errors,
write_errors,
flush_errors,
read_only,
Backend::RawFile,
log_request,
)
.await
}

// Build the downstairs struct given a region directory and some additional
// needed information. If a logger is passed in, we will use that, otherwise
// a logger will be created.
#[allow(clippy::too_many_arguments)]
pub async fn build_downstairs_for_region_with_backend(
data: &Path,
lossy: bool,
read_errors: bool,
write_errors: bool,
flush_errors: bool,
read_only: bool,
backend: Backend,
log_request: Option<Logger>,
) -> Result<Arc<Mutex<Downstairs>>> {
let log = match log_request {
Some(log) => log,
None => build_logger(),
};
let region = Region::open_with_backend(
data,
Default::default(),
true,
read_only,
backend,
&log,
)
.await?;

info!(log, "UUID: {:?}", region.def().uuid());
info!(
log,
"Blocks per extent:{} Total Extents: {}",
region.def().extent_size().value,
region.def().extent_count(),
);

let encrypted = region.encrypted();

Ok(Arc::new(Mutex::new(Downstairs::new(
region,
lossy,
read_errors,
write_errors,
flush_errors,
read_only,
encrypted,
log,
))))
}

/// Returns Ok if everything spawned ok, Err otherwise
///
/// Return Ok(main task join handle) if all the necessary tasks spawned
Expand Down Expand Up @@ -3568,16 +3571,10 @@ mod test {
region.extend(2).await?;

let path_dir = dir.as_ref().to_path_buf();
let ads = build_downstairs_for_region(
&path_dir,
false,
false,
false,
false,
false,
Some(csl()),
)
.await?;
let ads = Downstairs::new_builder(&path_dir, false)
.set_logger(csl())
.build()
.await?;

// This happens in proc() function.
let upstairs_connection = UpstairsConnection {
Expand Down Expand Up @@ -3653,16 +3650,10 @@ mod test {
region.extend(extent_count).await?;

let path_dir = dir.as_ref().to_path_buf();
let ads = build_downstairs_for_region(
&path_dir,
false,
false,
false,
false,
false,
Some(csl()),
)
.await?;
let ads = Downstairs::new_builder(&path_dir, false)
.set_logger(csl())
.build()
.await?;

Ok(ads)
}
Expand Down Expand Up @@ -5506,16 +5497,10 @@ mod test {

let path_dir = dir.as_ref().to_path_buf();

build_downstairs_for_region(
&path_dir,
false, // lossy
false, // read errors
false, // write errors
false, // flush errors
read_only,
Some(csl()),
)
.await
Downstairs::new_builder(&path_dir, read_only)
.set_logger(csl())
.build()
.await
}

#[tokio::test]
Expand Down Expand Up @@ -5915,16 +5900,10 @@ mod test {
region.extend(2).await?;

let path_dir = dir.as_ref().to_path_buf();
let ads = build_downstairs_for_region(
&path_dir,
false,
false,
false,
false,
false,
Some(csl()),
)
.await?;
let ads = Downstairs::new_builder(&path_dir, false)
.set_logger(csl())
.build()
.await?;

// This happens in proc() function.
let upstairs_connection_1 = UpstairsConnection {
Expand Down Expand Up @@ -6011,16 +5990,10 @@ mod test {
region.extend(2).await?;

let path_dir = dir.as_ref().to_path_buf();
let ads = build_downstairs_for_region(
&path_dir,
false,
false,
false,
false,
false,
Some(csl()),
)
.await?;
let ads = Downstairs::new_builder(&path_dir, false)
.set_logger(csl())
.build()
.await?;

// This happens in proc() function.
let upstairs_connection_1 = UpstairsConnection {
Expand Down Expand Up @@ -6107,16 +6080,10 @@ mod test {
region.extend(2).await?;

let path_dir = dir.as_ref().to_path_buf();
let ads = build_downstairs_for_region(
&path_dir,
false,
false,
false,
false,
false,
Some(csl()),
)
.await?;
let ads = Downstairs::new_builder(&path_dir, false)
.set_logger(csl())
.build()
.await?;

// This happens in proc() function.
let upstairs_connection_1 = UpstairsConnection {
Expand Down
Loading