Skip to content

Commit

Permalink
Improve freeing of pages
Browse files Browse the repository at this point in the history
Previously pages freed in the oldest transaction that still had a read
reference were not freed
  • Loading branch information
cberner committed Jul 26, 2024
1 parent b60ee0f commit 2169684
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 4 deletions.
9 changes: 5 additions & 4 deletions src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,9 +946,10 @@ impl WriteTransaction {
}

pub(crate) fn durable_commit(&mut self, eventual: bool, two_phase: bool) -> Result {
let oldest_live_read = self
let free_until_transaction = self
.transaction_tracker
.oldest_live_read_transaction()
.map(|x| x.next())
.unwrap_or(self.transaction_id);

let user_root = self
Expand All @@ -965,7 +966,7 @@ impl WriteTransaction {
.table_tree
.flush_table_root_updates()?;

self.process_freed_pages(oldest_live_read)?;
self.process_freed_pages(free_until_transaction)?;
// If a savepoint exists it might reference the freed-tree, since it holds a reference to the
// root of the freed-tree. Therefore, we must use the transactional free mechanism to free
// those pages. If there are no save points then these can be immediately freed, which is
Expand Down Expand Up @@ -1055,11 +1056,11 @@ impl WriteTransaction {

// NOTE: must be called before store_freed_pages() during commit, since this can create
// more pages freed by the current transaction
fn process_freed_pages(&mut self, oldest_live_read: TransactionId) -> Result {
fn process_freed_pages(&mut self, free_until: TransactionId) -> Result {
// We assume below that PageNumber is length 8
assert_eq!(PageNumber::serialized_size(), 8);
let lookup_key = FreedTableKey {
transaction_id: oldest_live_read.raw_id(),
transaction_id: free_until.raw_id(),
pagination_id: 0,
};

Expand Down
50 changes: 50 additions & 0 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,56 @@ fn regression21() {
drop(read_tx);
}

#[test]
fn regression22() {
let tmpfile = create_tempfile();

let db = Database::create(tmpfile.path()).unwrap();
let txn = db.begin_write().unwrap();
{
let mut table = txn.open_table(U64_TABLE).unwrap();
table.insert(0, 0).unwrap();
}
txn.commit().unwrap();

let txn = db.begin_write().unwrap();
{
let mut table = txn.open_table(U64_TABLE).unwrap();
table.remove(0).unwrap();
}
txn.commit().unwrap();

// Extra commit to finalize the cleanup of the freed pages
let txn = db.begin_write().unwrap();
txn.commit().unwrap();

let txn = db.begin_write().unwrap();
let allocated_pages = txn.stats().unwrap().allocated_pages();
{
let mut table = txn.open_table(U64_TABLE).unwrap();
table.insert(0, 0).unwrap();
}
txn.commit().unwrap();

let txn = db.begin_write().unwrap();
{
let mut table = txn.open_table(U64_TABLE).unwrap();
table.remove(0).unwrap();
}
txn.commit().unwrap();

let read_txn = db.begin_read().unwrap();

// Extra commit to finalize the cleanup of the freed pages. The read transaction should not
// block the freeing, but there was a bug where it did.
db.begin_write().unwrap().commit().unwrap();

drop(read_txn);

let txn = db.begin_write().unwrap();
assert_eq!(allocated_pages, txn.stats().unwrap().allocated_pages());
}

#[test]
fn check_integrity_clean() {
let tmpfile = create_tempfile();
Expand Down

0 comments on commit 2169684

Please sign in to comment.