diff --git a/firewood/src/manager.rs b/firewood/src/manager.rs index 4753c8dca..41875644f 100644 --- a/firewood/src/manager.rs +++ b/firewood/src/manager.rs @@ -143,14 +143,14 @@ impl RevisionManager { return Err(RevisionManagerError::NotLatest); } - let mut committed = proposal.as_committed(); + let committed = proposal.as_committed(); // 2. Persist delete list for this committed revision to disk for recovery // 3 Take the deleted entries from the oldest revision and mark them as free for this revision // If you crash after freeing some of these, then the free list will point to nodes that are not actually free. // TODO: Handle the case where we get something off the free list that is not free - while self.historical.len() > self.max_revisions { + while self.historical.len() >= self.max_revisions { let oldest = self.historical.pop_front().expect("must be present"); if let Some(oldest_hash) = oldest.kind.root_hash() { self.by_hash.remove(&oldest_hash); @@ -162,9 +162,8 @@ impl RevisionManager { // This guarantee is there because we have a `&mut self` reference to the manager, so // the compiler guarantees we are the only one using this manager. match Arc::try_unwrap(oldest) { - Ok(oldest) => committed.reap_deleted(&oldest)?, + Ok(oldest) => oldest.reap_deleted()?, Err(original) => { - // TODO: try reaping the next revision warn!("Oldest revision could not be reaped; still referenced"); self.historical.push_front(original); } diff --git a/storage/src/nodestore.rs b/storage/src/nodestore.rs index 6bf1b9955..3d8040e03 100644 --- a/storage/src/nodestore.rs +++ b/storage/src/nodestore.rs @@ -53,7 +53,7 @@ use std::fmt::Debug; /// ``` use std::io::{Error, ErrorKind, Write}; use std::iter::once; -use std::mem::offset_of; +use std::mem::{offset_of, take}; use std::num::NonZeroU64; use std::ops::Deref; use std::sync::Arc; @@ -1092,12 +1092,12 @@ where impl NodeStore { /// adjust the freelist of this proposal to reflect the freed nodes in the oldest proposal - pub fn reap_deleted(&mut self, oldest: &NodeStore) -> Result<(), Error> { + pub fn reap_deleted(mut self) -> Result<(), Error> { self.storage - .invalidate_cached_nodes(oldest.kind.deleted.iter()); - trace!("There are {} nodes to reap", oldest.kind.deleted.len()); - for addr in oldest.kind.deleted.iter() { - self.delete_node(*addr)?; + .invalidate_cached_nodes(self.kind.deleted.iter()); + trace!("There are {} nodes to reap", self.kind.deleted.len()); + for addr in take(&mut self.kind.deleted) { + self.delete_node(addr)?; } Ok(()) }