Skip to content

Commit

Permalink
Fix region tracker page leak and initial sizing
Browse files Browse the repository at this point in the history
When allocating a new region tracker, we need to free the old one.
Also, when recreating the region tracker during repair, it has to be
large enough for the actual number of regions (which might be more
than INITIAL_REGIONS).
  • Loading branch information
mconst authored and cberner committed Nov 1, 2024
1 parent 9ffa1d1 commit e413256
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 7 deletions.
16 changes: 11 additions & 5 deletions src/tree_store/page_store/page_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,13 +1023,11 @@ impl Drop for TransactionalMemory {
}
let mut state = self.state.lock().unwrap();
let tracker_len = state.allocators.region_tracker.to_vec().len();
let tracker_page_size = state
.header
.region_tracker()
.page_size_bytes(self.page_size);
if tracker_page_size < (tracker_len as u64) {
let tracker_page = state.header.region_tracker();
if tracker_page.page_size_bytes(self.page_size) < (tracker_len as u64) {
drop(state);
// Allocate a larger tracker page
self.free(tracker_page);
if let Ok(tracker_page) = self.allocate(tracker_len) {
state = self.state.lock().unwrap();
state
Expand Down Expand Up @@ -1090,5 +1088,13 @@ mod test {
}
}
txn.commit().unwrap();
drop(db);

let mut db = Database::builder()
.set_region_size((8 * page_size).try_into().unwrap())
.set_page_size(page_size)
.open(tmpfile.path())
.unwrap();
assert!(db.check_integrity().unwrap());
}
}
5 changes: 3 additions & 2 deletions src/tree_store/page_store/region.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::tree_store::page_store::page_manager::{INITIAL_REGIONS, MAX_MAX_PAGE_
use crate::tree_store::page_store::xxh3_checksum;
use crate::tree_store::PageNumber;
use crate::Result;
use std::cmp;
use std::cmp::{self, max};
use std::mem::size_of;

const REGION_FORMAT_VERSION: u8 = 1;
Expand Down Expand Up @@ -133,7 +133,8 @@ pub(super) struct Allocators {
impl Allocators {
pub(super) fn new(layout: DatabaseLayout) -> Self {
let mut region_allocators = vec![];
let mut region_tracker = RegionTracker::new(INITIAL_REGIONS, MAX_MAX_PAGE_ORDER + 1);
let initial_regions = max(INITIAL_REGIONS, layout.num_regions());
let mut region_tracker = RegionTracker::new(initial_regions, MAX_MAX_PAGE_ORDER + 1);
for i in 0..layout.num_regions() {
let region_layout = layout.region_layout(i);
let allocator = BuddyAllocator::new(
Expand Down

0 comments on commit e413256

Please sign in to comment.