From 7ff12bb7925e311748d08601eb3240d914aef0c7 Mon Sep 17 00:00:00 2001 From: Michael Constant Date: Mon, 18 Nov 2024 02:38:36 -0800 Subject: [PATCH] Compaction also needs to use allocate_non_transactional() If we relocate the region tracker page during compaction and then the transaction aborts, the allocation of the new tracker page gets rolled back, but the free of the old tracker page doesn't! This leaves us in an inconsistent state with no tracker page allocated. So we need to use allocate_non_transactional() here, which guarantees the page will remain allocated even if the rest of the transaction rolls back. --- src/tree_store/page_store/page_manager.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/tree_store/page_store/page_manager.rs b/src/tree_store/page_store/page_manager.rs index b441602b..f0dd6631 100644 --- a/src/tree_store/page_store/page_manager.rs +++ b/src/tree_store/page_store/page_manager.rs @@ -539,7 +539,7 @@ impl TransactionalMemory { // Allocate a larger tracker page self.free(tracker_page); tracker_page = self - .allocate_non_transactional(tracker_len)? + .allocate_non_transactional(tracker_len, false)? .get_page_number(); let mut state = self.state.lock().unwrap(); state.header.set_region_tracker(tracker_page); @@ -559,10 +559,8 @@ impl TransactionalMemory { let old_tracker_page = state.header.region_tracker(); // allocate acquires this lock, so we need to drop it drop(state); - // Allocate the new page. Unlike other region-tracker allocations, this happens inside - // a transaction, so we use an ordinary allocation (which gets committed or rolled back - // along with the rest of the transaction) rather than allocate_non_transactional() - let new_page = self.allocate_lowest(region_tracker_size.try_into().unwrap())?; + let new_page = + self.allocate_non_transactional(region_tracker_size.try_into().unwrap(), true)?; if new_page.get_page_number().is_before(old_tracker_page) { let mut state = self.state.lock().unwrap(); state.header.set_region_tracker(new_page.get_page_number()); @@ -1110,8 +1108,8 @@ impl TransactionalMemory { // Allocate a page not associated with any transaction. The page is immediately considered committed, // and won't be rolled back if an abort happens. This is only used for the region tracker - fn allocate_non_transactional(&self, allocation_size: usize) -> Result { - self.allocate_helper(allocation_size, false, false) + fn allocate_non_transactional(&self, allocation_size: usize, lowest: bool) -> Result { + self.allocate_helper(allocation_size, lowest, false) } pub(crate) fn count_allocated_pages(&self) -> Result {