Skip to content

Commit

Permalink
Refactor InternalTableDefinition
Browse files Browse the repository at this point in the history 
This makes it harder to misuse and uncovered one bug in the compaction
code already
  • Loading branch information
@cberner
cberner committed Aug 25, 2024
1 parent 7a35233 commit c8bcd2d
Show file tree
Hide file tree
Showing 5 changed files with 724 additions and 617 deletions.
127 changes: 16 additions & 111 deletions src/db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use std::path::Path;
use std::sync::{Arc, Mutex};

use crate::error::TransactionError;
use crate::multimap_table::{parse_subtree_roots, DynamicCollection};
use crate::sealed::Sealed;
use crate::transactions::SAVEPOINT_TABLE;
use crate::tree_store::file_backend::FileBackend;
Expand Down Expand Up @@ -483,10 +482,16 @@ impl Database {
e.into_storage_error_or_corrupted("Persistent savepoint table corrupted")
})?
{
let savepoint_table_root =
if let InternalTableDefinition::Normal { table_root, .. } = savepoint_table_def {
table_root
} else {
unreachable!()
};
let savepoint_table: ReadOnlyTable<SavepointId, SerializedSavepoint> =
ReadOnlyTable::new(
"internal savepoint table".to_string(),
savepoint_table_def.get_root(),
savepoint_table_root,
PageHint::None,
Arc::new(TransactionGuard::fake()),
mem.clone(),
Expand Down Expand Up @@ -563,65 +568,10 @@ impl Database {
// Chain all the other tables to the master table iter
for entry in iter {
let definition = entry?.value();
if let Some(header) = definition.get_root() {
match definition.get_type() {
TableType::Normal => {
let table_pages_iter = AllPageNumbersBtreeIter::new(
header.root,
definition.get_fixed_key_size(),
definition.get_fixed_value_size(),
mem.clone(),
)?;
for result in table_pages_iter {
let page = result?;
assert!(mem.is_allocated(page));
}
}
TableType::Multimap => {
let table_pages_iter = AllPageNumbersBtreeIter::new(
header.root,
definition.get_fixed_key_size(),
DynamicCollection::<()>::fixed_width_with(
definition.get_fixed_value_size(),
),
mem.clone(),
)?;
for result in table_pages_iter {
let page = result?;
assert!(mem.is_allocated(page));
}

let table_pages_iter = AllPageNumbersBtreeIter::new(
header.root,
definition.get_fixed_key_size(),
DynamicCollection::<()>::fixed_width_with(
definition.get_fixed_value_size(),
),
mem.clone(),
)?;
for table_page in table_pages_iter {
let page = mem.get_page(table_page?)?;
let subtree_roots = parse_subtree_roots(
&page,
definition.get_fixed_key_size(),
definition.get_fixed_value_size(),
);
for subtree_header in subtree_roots {
let sub_root_iter = AllPageNumbersBtreeIter::new(
subtree_header.root,
definition.get_fixed_value_size(),
<()>::fixed_width(),
mem.clone(),
)?;
for result in sub_root_iter {
let page = result?;
assert!(mem.is_allocated(page));
}
}
}
}
}
}
definition.visit_all_pages(mem.clone(), |page_number| {
assert!(mem.is_allocated(page_number));
Ok(())
})?;
}

Ok(())
Expand All @@ -644,56 +594,11 @@ impl Database {
// Chain all the other tables to the master table iter
for entry in iter {
let definition = entry?.value();
if let Some(header) = definition.get_root() {
match definition.get_type() {
TableType::Normal => {
let table_pages_iter = AllPageNumbersBtreeIter::new(
header.root,
definition.get_fixed_key_size(),
definition.get_fixed_value_size(),
mem.clone(),
)?;
mem.mark_pages_allocated(table_pages_iter, allow_duplicates)?;
}
TableType::Multimap => {
let table_pages_iter = AllPageNumbersBtreeIter::new(
header.root,
definition.get_fixed_key_size(),
DynamicCollection::<()>::fixed_width_with(
definition.get_fixed_value_size(),
),
mem.clone(),
)?;
mem.mark_pages_allocated(table_pages_iter, allow_duplicates)?;

let table_pages_iter = AllPageNumbersBtreeIter::new(
header.root,
definition.get_fixed_key_size(),
DynamicCollection::<()>::fixed_width_with(
definition.get_fixed_value_size(),
),
mem.clone(),
)?;
for table_page in table_pages_iter {
let page = mem.get_page(table_page?)?;
let subtree_roots = parse_subtree_roots(
&page,
definition.get_fixed_key_size(),
definition.get_fixed_value_size(),
);
for subtree_header in subtree_roots {
let sub_root_iter = AllPageNumbersBtreeIter::new(
subtree_header.root,
definition.get_fixed_value_size(),
<()>::fixed_width(),
mem.clone(),
)?;
mem.mark_pages_allocated(sub_root_iter, allow_duplicates)?;
}
}
}
}
}
definition.visit_all_pages(mem.clone(), |page_number| {
// TODO: simplify mark_pages_allocated()
mem.mark_pages_allocated([Ok(page_number)].into_iter(), allow_duplicates)?;
Ok(())
})?;
}

Ok(())
Expand Down
89 changes: 58 additions & 31 deletions src/transactions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<'db> SystemNamespace<'db> {
) -> Result<SystemTable<'db, 's, K, V>> {
#[cfg(feature = "logging")]
debug!("Opening system table: {}", definition);
let root = self
let (root, _) = self
.table_tree
.get_or_create_table::<K, V>(definition.name(), TableType::Normal)
.map_err(|e| {
Expand All @@ -296,7 +296,7 @@ impl<'db> SystemNamespace<'db> {

Ok(SystemTable::new(
definition.name(),
root.get_root(),
root,
transaction.freed_pages.clone(),
self.transaction_guard.clone(),
transaction.mem.clone(),
Expand Down Expand Up @@ -331,13 +331,13 @@ impl<'db> TableNamespace<'db> {
return Err(TableError::TableAlreadyOpen(name.to_string(), location));
}

let internal_table = self
let root = self
.table_tree
.get_or_create_table::<K, V>(name, table_type)?;
self.open_tables
.insert(name.to_string(), panic::Location::caller());

Ok((internal_table.get_root(), internal_table.get_length()))
Ok(root)
}

#[track_caller]
Expand Down Expand Up @@ -1338,13 +1338,16 @@ impl ReadTransaction {
.get_table::<K, V>(definition.name(), TableType::Normal)?
.ok_or_else(|| TableError::TableDoesNotExist(definition.name().to_string()))?;

Ok(ReadOnlyTable::new(
definition.name().to_string(),
header.get_root(),
PageHint::Clean,
self.tree.transaction_guard().clone(),
self.mem.clone(),
)?)
match header {
InternalTableDefinition::Normal { table_root, .. } => Ok(ReadOnlyTable::new(
definition.name().to_string(),
table_root,
PageHint::Clean,
self.tree.transaction_guard().clone(),
self.mem.clone(),
)?),
InternalTableDefinition::Multimap { .. } => unreachable!(),
}
}

/// Open the given table without a type
Expand All @@ -1357,12 +1360,20 @@ impl ReadTransaction {
.get_table_untyped(handle.name(), TableType::Normal)?
.ok_or_else(|| TableError::TableDoesNotExist(handle.name().to_string()))?;

Ok(ReadOnlyUntypedTable::new(
header.get_root(),
header.get_fixed_key_size(),
header.get_fixed_value_size(),
self.mem.clone(),
))
match header {
InternalTableDefinition::Normal {
table_root,
fixed_key_size,
fixed_value_size,
..
} => Ok(ReadOnlyUntypedTable::new(
table_root,
fixed_key_size,
fixed_value_size,
self.mem.clone(),
)),
InternalTableDefinition::Multimap { .. } => unreachable!(),
}
}

/// Open the given table
Expand All @@ -1375,13 +1386,20 @@ impl ReadTransaction {
.get_table::<K, V>(definition.name(), TableType::Multimap)?
.ok_or_else(|| TableError::TableDoesNotExist(definition.name().to_string()))?;

Ok(ReadOnlyMultimapTable::new(
header.get_root(),
header.get_length(),
PageHint::Clean,
self.tree.transaction_guard().clone(),
self.mem.clone(),
)?)
match header {
InternalTableDefinition::Normal { .. } => unreachable!(),
InternalTableDefinition::Multimap {
table_root,
table_length,
..
} => Ok(ReadOnlyMultimapTable::new(
table_root,
table_length,
PageHint::Clean,
self.tree.transaction_guard().clone(),
self.mem.clone(),
)?),
}
}

/// Open the given table without a type
Expand All @@ -1394,13 +1412,22 @@ impl ReadTransaction {
.get_table_untyped(handle.name(), TableType::Multimap)?
.ok_or_else(|| TableError::TableDoesNotExist(handle.name().to_string()))?;

Ok(ReadOnlyUntypedMultimapTable::new(
header.get_root(),
header.get_length(),
header.get_fixed_key_size(),
header.get_fixed_value_size(),
self.mem.clone(),
))
match header {
InternalTableDefinition::Normal { .. } => unreachable!(),
InternalTableDefinition::Multimap {
table_root,
table_length,
fixed_key_size,
fixed_value_size,
..
} => Ok(ReadOnlyUntypedMultimapTable::new(
table_root,
table_length,
fixed_key_size,
fixed_value_size,
self.mem.clone(),
)),
}
}

/// List all the tables
Expand Down
6 changes: 3 additions & 3 deletions src/tree_store/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod btree_iters;
mod btree_mutator;
mod page_store;
mod table_tree;
mod table_tree_base;

pub(crate) use btree::{btree_stats, Btree, BtreeMut, BtreeStats, RawBtree, UntypedBtreeMut};
pub use btree_base::{AccessGuard, AccessGuardMut};
Expand All @@ -16,6 +17,5 @@ pub(crate) use page_store::{
CachePriority, Page, PageHint, PageNumber, SerializedSavepoint, TransactionalMemory,
FILE_FORMAT_VERSION2, MAX_PAIR_LENGTH, MAX_VALUE_LENGTH, PAGE_SIZE,
};
pub(crate) use table_tree::{
FreedPageList, FreedTableKey, InternalTableDefinition, TableTree, TableTreeMut, TableType,
};
pub(crate) use table_tree::{FreedPageList, FreedTableKey, TableTree, TableTreeMut};
pub(crate) use table_tree_base::{InternalTableDefinition, TableType};
Loading

0 comments on commit c8bcd2d

Please sign in to comment.