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

Revert allocation transaction committed #3491

Merged
merged 1 commit into from
Nov 16, 2023
+95 −363
Merged

Revert allocation transaction committed #3491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 25 additions & 45 deletions src/engine/strat_engine/backstore/backstore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use crate::{
data_tier::DataTier,
devices::UnownedDevices,
shared::BlockSizeSummary,
transaction::RequestTransaction,
},
dm::{get_dm, list_of_backstore_devices, remove_optional_devices},
metadata::{MDADataSize, BDA},
Expand Down Expand Up @@ -416,59 +415,41 @@ impl Backstore {
/// retained as a convenience to the caller.
/// Postcondition:
/// forall i, result_i.0 = result_(i - 1).0 + result_(i - 1).1
pub fn request_alloc(
///
/// WARNING: metadata changing event
pub fn alloc(
&mut self,
pool_uuid: PoolUuid,
sizes: &[Sectors],
) -> StratisResult<Option<RequestTransaction>> {
let mut transaction = match self.data_tier.alloc_request(sizes)? {
Some(t) => t,
None => return Ok(None),
};
) -> StratisResult<Option<Vec<(Sectors, Sectors)>>> {
let total_required = sizes.iter().cloned().sum();
if self.available_in_backstore() < total_required {
return Ok(None);
}

let mut next = self.next;
if self.data_tier.alloc(sizes) {
self.extend_cap_device(pool_uuid)?;
} else {
return Ok(None);
}

let mut chunks = Vec::new();
for size in sizes {
transaction.add_seg_req((next, *size));
next += *size
chunks.push((self.next, *size));
self.next += *size;
}

// Assert that the postcondition holds.
assert_eq!(
sizes,
transaction
.get_backstore()
chunks
.iter()
.map(|x| x.1)
.collect::<Vec<Sectors>>()
.as_slice()
);

Ok(Some(transaction))
}

/// Commit space requested by request_alloc() to metadata.
///
/// This method commits the newly allocated data segments and then extends the cap device
/// to be the same size as the allocated data size.
pub fn commit_alloc(
&mut self,
pool_uuid: PoolUuid,
transaction: RequestTransaction,
) -> StratisResult<()> {
let segs = transaction.get_backstore();
self.data_tier.alloc_commit(transaction)?;
// This must occur after the segments have been updated in the data tier
self.extend_cap_device(pool_uuid)?;

assert!(self.next <= self.size());

self.next += segs
.into_iter()
.fold(Sectors(0), |mut size, (_, next_size)| {
size += next_size;
size
});

Ok(())
Ok(Some(chunks))
}

/// Get only the datadevs in the pool.
Expand Down Expand Up @@ -542,6 +523,7 @@ impl Backstore {
/// DM devices. But the devicemapper library stores the data from which
/// the size of each DM device is calculated; the result is computed and
/// no ioctl is required.
#[cfg(test)]
fn size(&self) -> Sectors {
self.linear
.as_ref()
Expand Down Expand Up @@ -1179,11 +1161,10 @@ mod tests {
invariant(&backstore);

// Allocate space from the backstore so that the cap device is made.
let transaction = backstore
.request_alloc(&[INITIAL_BACKSTORE_ALLOCATION])
backstore
.alloc(pool_uuid, &[INITIAL_BACKSTORE_ALLOCATION])
.unwrap()
.unwrap();
backstore.commit_alloc(pool_uuid, transaction).unwrap();

let cache_uuids = backstore
.init_cache(pool_name.clone(), pool_uuid, initcachedevs, None)
Expand Down Expand Up @@ -1295,11 +1276,10 @@ mod tests {
invariant(&backstore);

// Allocate space from the backstore so that the cap device is made.
let transaction = backstore
.request_alloc(&[INITIAL_BACKSTORE_ALLOCATION])
backstore
.alloc(pool_uuid, &[INITIAL_BACKSTORE_ALLOCATION])
.unwrap()
.unwrap();
backstore.commit_alloc(pool_uuid, transaction).unwrap();

let old_device = backstore.device();

Expand Down
16 changes: 2 additions & 14 deletions src/engine/strat_engine/backstore/blockdev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use crate::{
crypt::CryptHandle,
devices::BlockSizes,
range_alloc::{PerDevSegments, RangeAllocator},
transaction::RequestTransaction,
},
device::blkdev_size,
metadata::{
Expand Down Expand Up @@ -267,19 +266,8 @@ impl StratBlockDev {

/// Find some sector ranges that could be allocated. If more
/// sectors are needed than are available, return partial results.
pub fn request_space(
&self,
size: Sectors,
transaction: &RequestTransaction,
) -> StratisResult<PerDevSegments> {
self.used.request(self.uuid(), size, transaction)
}

/// Commit allocation requested by request_space().
///
/// This method will record the requested allocations in the metadata.
pub fn commit_space(&mut self, segs: PerDevSegments) {
self.used.commit(segs);
pub fn alloc(&mut self, size: Sectors) -> PerDevSegments {
self.used.alloc(size)
}

// ALL SIZE METHODS (except size(), which is in BlockDev impl.)
Expand Down
72 changes: 15 additions & 57 deletions src/engine/strat_engine/backstore/blockdevmgr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ use crate::{
blockdev::StratBlockDev,
crypt::CryptHandle,
devices::{initialize_devices, wipe_blockdevs, UnownedDevices},
range_alloc::PerDevSegments,
shared::{BlkDevSegment, Segment},
transaction::RequestTransaction,
},
metadata::{MDADataSize, BDA},
serde_structs::{BaseBlockDevSave, Recordable},
Expand Down Expand Up @@ -194,80 +192,41 @@ impl BlockDevMgr {
/// not possible to satisfy the request.
/// This method is atomic, it either allocates all requested or allocates
/// nothing.
pub fn request_space(&self, sizes: &[Sectors]) -> StratisResult<Option<RequestTransaction>> {
let mut transaction = RequestTransaction::default();

pub fn alloc(&mut self, sizes: &[Sectors]) -> Option<Vec<Vec<BlkDevSegment>>> {
let total_needed: Sectors = sizes.iter().cloned().sum();
if self.avail_space() < total_needed {
return Ok(None);
return None;
}

for (idx, &needed) in sizes.iter().enumerate() {
let mut lists = Vec::new();

for &needed in sizes.iter() {
let mut alloc = Sectors(0);
let mut segs = Vec::new();
// TODO: Consider greater efficiency for allocation generally.
// Over time, the blockdevs at the start will be exhausted. It
// might be a good idea to keep an auxiliary structure, so that
// only blockdevs with some space left to allocate are accessed.
// In the context of this major inefficiency that ensues over time
// the obvious but more minor inefficiency of this inner loop is
// not worth worrying about.
for bd in &self.block_devs {
for bd in &mut self.block_devs {
if alloc == needed {
break;
}

let r_segs = bd.request_space(needed - alloc, &transaction)?;
for (&start, &length) in r_segs.iter() {
transaction.add_bd_seg_req(
idx,
BlkDevSegment::new(bd.uuid(), Segment::new(*bd.device(), start, length)),
);
}
let r_segs = bd.alloc(needed - alloc);
let blkdev_segs = r_segs.iter().map(|(&start, &length)| {
BlkDevSegment::new(bd.uuid(), Segment::new(*bd.device(), start, length))
});
segs.extend(blkdev_segs);
alloc += r_segs.sum();
}
assert_eq!(alloc, needed);
lists.push(segs);
}

Ok(Some(transaction))
}

/// Commit the allocations calculated by the request_space() method.
///
/// This method converts the block device segments into the necessary data
/// structure and dispatches them to the corresponding block devices to
/// update the internal records of allocated space.
pub fn commit_space(&mut self, mut transaction: RequestTransaction) -> StratisResult<()> {
let mut segs = transaction.drain_blockdevmgr().try_fold(
HashMap::<DevUuid, PerDevSegments>::new(),
|mut map, seg| -> StratisResult<_> {
if let Some(segs) = map.get_mut(&seg.uuid) {
segs.insert(&(seg.segment.start, seg.segment.length))?;
} else {
let mut segs = PerDevSegments::new(
self.block_devs
.iter()
.find(|bd| bd.uuid() == seg.uuid)
.expect(
"Block dev was determined to be present during allocation request",
)
.total_size()
.sectors(),
);
segs.insert(&(seg.segment.start, seg.segment.length))?;
map.insert(seg.uuid, segs);
}

Ok(map)
},
)?;

for (uuid, bd) in self.blockdevs_mut() {
if let Some(segs) = segs.remove(&uuid) {
bd.commit_space(segs);
}
}

Ok(())
Some(lists)
}

/// Write the given data to all blockdevs marking with current time.
Expand Down Expand Up @@ -472,8 +431,7 @@ mod tests {
assert_eq!(mgr.avail_space() + mgr.metadata_size(), mgr.size());

let allocated = Sectors(2);
let transaction = mgr.request_space(&[allocated]).unwrap().unwrap();
mgr.commit_space(transaction).unwrap();
mgr.alloc(&[allocated]).unwrap();
assert_eq!(
mgr.avail_space() + allocated + mgr.metadata_size(),
mgr.size()
Expand Down
35 changes: 13 additions & 22 deletions src/engine/strat_engine/backstore/cache_tier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,17 +128,14 @@ impl CacheTier {
)));
}

let trans = self
let segments = self
.block_mgr
.request_space(&[avail_space])?
.expect("asked for exactly the space available, must get");
let segments = trans.get_blockdevmgr();
if let Err(e) = self.block_mgr.commit_space(trans) {
self.block_mgr.remove_blockdevs(&uuids)?;
return Err(StratisError::Msg(format!(
"Failed to commit metadata changes: {e}"
)));
}
.alloc(&[avail_space])
.expect("asked for exactly the space available, must get")
.iter()
.flat_map(|s| s.iter())
.cloned()
.collect::<Vec<_>>();
self.cache_segments.coalesce_blkdevsegs(&segments);

Ok((uuids, (true, false)))
Expand Down Expand Up @@ -170,21 +167,15 @@ impl CacheTier {
)));
}

let trans = block_mgr
.request_space(&[meta_space, avail_space - meta_space])?
let mut segments = block_mgr
.alloc(&[meta_space, avail_space - meta_space])
.expect("asked for exactly the space available, must get");
let meta_segments = AllocatedAbove {
inner: trans.get_segs_for_req(0).expect("segments.len() == 2"),
};
let cache_segments = AllocatedAbove {
inner: trans.get_segs_for_req(1).expect("segments.len() == 2"),
inner: segments.pop().expect("segments.len() == 2"),
};
let meta_segments = AllocatedAbove {
inner: segments.pop().expect("segments.len() == 1"),
};
if let Err(e) = block_mgr.commit_space(trans) {
block_mgr.destroy_all()?;
return Err(StratisError::Msg(format!(
"Failed to commit metadata changes: {e}"
)));
}

Ok(CacheTier {
block_mgr,
Expand Down
40 changes: 18 additions & 22 deletions src/engine/strat_engine/backstore/data_tier.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use crate::{
blockdevmgr::BlockDevMgr,
devices::UnownedDevices,
shared::{metadata_to_segment, AllocatedAbove, BlkDevSegment, BlockDevPartition},
transaction::RequestTransaction,
},
serde_structs::{BaseDevSave, BlockDevSave, DataTierSave, Recordable},
types::BDARecordResult,
Expand Down Expand Up @@ -89,20 +88,22 @@ impl DataTier {
}

/// Allocate a region for all sector size requests from unallocated segments in
/// block devices belonging to the data tier. Return Some(_) if requested
/// amount or more was allocated, otherwise, None.
pub fn alloc_request(&self, requests: &[Sectors]) -> StratisResult<Option<RequestTransaction>> {
self.block_mgr.request_space(requests)
}

/// Commit an allocation that was determined to be valid by alloc_request()
/// to metadata.
pub fn alloc_commit(&mut self, transaction: RequestTransaction) -> StratisResult<()> {
let segments = transaction.get_blockdevmgr();
self.block_mgr.commit_space(transaction)?;
self.segments.coalesce_blkdevsegs(&segments);

Ok(())
/// block devices belonging to the data tier. Return true if requested
/// amount or more was allocated, otherwise, false.
pub fn alloc(&mut self, requests: &[Sectors]) -> bool {
self.block_mgr
.alloc(requests)
.map(|segments| {
self.segments.coalesce_blkdevsegs(
&segments
.iter()
.flat_map(|s| s.iter())
.cloned()
.collect::<Vec<_>>(),
);
true
})
.unwrap_or(false)
}

/// The sum of the lengths of all the sectors that have been mapped to an
Expand Down Expand Up @@ -260,8 +261,7 @@ mod tests {
let request_amount = data_tier.block_mgr.avail_space() / 2usize;
assert!(request_amount != Sectors(0));

let transaction = data_tier.alloc_request(&[request_amount]).unwrap().unwrap();
data_tier.alloc_commit(transaction).unwrap();
assert!(data_tier.alloc(&[request_amount]));
data_tier.invariant();

// A data tier w/ some amount allocated
Expand All @@ -279,11 +279,7 @@ mod tests {
size = data_tier.size();

// Allocate enough to get into the newly added block devices
let transaction = data_tier
.alloc_request(&[last_request_amount])
.unwrap()
.unwrap();
data_tier.alloc_commit(transaction).unwrap();
assert!(data_tier.alloc(&[last_request_amount]));
data_tier.invariant();

assert!(data_tier.allocated() >= request_amount + last_request_amount);
Expand Down
1 change: 0 additions & 1 deletion src/engine/strat_engine/backstore/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ mod data_tier;
mod devices;
mod range_alloc;
mod shared;
mod transaction;

pub use self::{
backstore::Backstore,
Expand Down
Loading