Skip to content

Commit

Permalink
Merge pull request #3491 from jbaublitz/revert-transaction
Browse files Browse the repository at this point in the history 
Revert allocation transaction committed
  • Loading branch information
@mulkieran
mulkieran authored Nov 16, 2023
2 parents 3ff45d8 + 1c0cd0d commit bb5f634
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 363 deletions.
70 changes: 25 additions & 45 deletions src/engine/strat_engine/backstore/backstore.rs
View file
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
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
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
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
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
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

0 comments on commit bb5f634

Please sign in to comment.