Skip to content

Commit

Permalink
chore: address some easy and outdated TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
rphmeier committed Aug 24, 2024
1 parent 806f367 commit 1ab58d6
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 35 deletions.
4 changes: 3 additions & 1 deletion nomt/src/beatree/allocator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ impl AllocatorReader {
user_data: 0,
};

self.io_handle.send(command).expect("I/O store worker dropped");
self.io_handle
.send(command)
.expect("I/O store worker dropped");

// wait for completion
let completion = self.io_handle.recv().expect("I/O store worker dropped");
Expand Down
4 changes: 1 addition & 3 deletions nomt/src/beatree/branch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ use std::{
sync::{Arc, Mutex},
};

pub use node::{
body_size, BranchNode, BranchNodeBuilder, BranchNodeView, BRANCH_NODE_BODY_SIZE,
};
pub use node::{body_size, BranchNode, BranchNodeBuilder, BranchNodeView, BRANCH_NODE_BODY_SIZE};

pub mod node;

Expand Down
10 changes: 0 additions & 10 deletions nomt/src/beatree/branch/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,6 @@ impl BranchNode {
let offset = BRANCH_NODE_SIZE - (self.n() as usize - i) * 4;
self.as_mut_slice()[offset..offset + 4].copy_from_slice(&node_pointer.to_le_bytes());
}

// TODO: modification.
//
// Coming up with the right API for this is tricky:
// - The offsets depend on `n`, `prefix_len` and `separator_len`, which suggests that it should
// be supplied all at once.
// - At the same time, we want to avoid materializing the structure in memory.
//
// It all depends on how the caller wants to use this. Ideally, the caller would be able to
// build the new node in a single pass.
}

impl Drop for BranchNode {
Expand Down
20 changes: 13 additions & 7 deletions nomt/src/bitbox/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use anyhow::Context;
use crossbeam::channel::{TryRecvError, TrySendError};
use nomt_core::page_id::PageId;
use parking_lot::{ArcRwLockReadGuard, RwLock};
Expand Down Expand Up @@ -115,7 +116,6 @@ impl DB {
}
}

// TODO: update with async sync apporach
pub fn sync_begin(
&self,
changes: Vec<(PageId, BucketIndex, Option<(Vec<u8>, PageDiff)>)>,
Expand Down Expand Up @@ -196,8 +196,10 @@ impl DB {
),
user_data: 0, // unimportant.
};
// TODO: handle error
self.shared.io_handle.send(command).unwrap();
self.shared
.io_handle
.send(command)
.map_err(|_| anyhow::anyhow!("I/O Pool Disconnected"))?;
submitted += 1;
}

Expand All @@ -214,19 +216,23 @@ impl DB {
user_data: 0, // unimportant
};
submitted += 1;
// TODO: handle error
self.shared.io_handle.send(command).unwrap();
self.shared
.io_handle
.send(command)
.map_err(|_| anyhow::anyhow!("I/O Pool Disconnected"))?;
}

// wait for all writes command to be finished
while completed < submitted {
let completion = self.shared.io_handle.recv().expect("I/O worker dropped");
let completion = self.shared.io_handle.recv()?;
assert!(completion.result.is_ok());
completed += 1;
}

// sync all writes
ht_fd.sync_all().expect("ht file: error performing fsync");
ht_fd
.sync_all()
.context("ht file: error performing fsync")?;

Ok(prev_wal_size)
}
Expand Down
6 changes: 1 addition & 5 deletions nomt/src/io/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,7 @@ fn run_worker(command_rx: Receiver<IoPacket>) {
};

let complete = CompleteIo { command, result };

if let Err(_) = completion_sender.send(complete) {
// TODO: handle?
break;
}
let _ = completion_sender.send(complete);
}
}

Expand Down
11 changes: 8 additions & 3 deletions nomt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use commit::{CommitPool, Committer};
use nomt_core::{
page_id::ROOT_PAGE_ID,
proof::PathProof,
trie::{NodeHasher, NodeHasherExt, InternalData, ValueHash, TERMINATOR},
trie::{InternalData, NodeHasher, NodeHasherExt, ValueHash, TERMINATOR},
};
use page_cache::PageCache;
use parking_lot::Mutex;
Expand Down Expand Up @@ -374,7 +374,9 @@ impl Drop for Session {
}

fn compute_root_node<H: NodeHasher>(page_cache: &PageCache) -> Node {
let Some(root_page) = page_cache.get(ROOT_PAGE_ID) else { return TERMINATOR };
let Some(root_page) = page_cache.get(ROOT_PAGE_ID) else {
return TERMINATOR;
};
let read_pass = page_cache.new_read_pass();

// 3 cases.
Expand All @@ -390,7 +392,10 @@ fn compute_root_node<H: NodeHasher>(page_cache: &PageCache) -> Node {
if is_empty(0) && is_empty(1) {
TERMINATOR
} else if (2..6usize).all(is_empty) {
H::hash_leaf(&LeafData { key_path: left, value_hash: right })
H::hash_leaf(&LeafData {
key_path: left,
value_hash: right,
})
} else {
H::hash_internal(&InternalData { left, right })
}
Expand Down
2 changes: 0 additions & 2 deletions nomt/src/page_walker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,6 @@ mod tests {
fn advance_backwards_panics() {
let root = trie::TERMINATOR;
let page_cache = PageCache::new(None, &crate::Options::new(), None);
// TODO: prepopulate page cache with everything on the path? or just mock it somehow
// so our `get`s don't fail...

let mut walker = PageWalker::<Blake3Hasher>::new(root, page_cache.clone(), None);
let mut write_pass = page_cache.new_write_pass();
Expand Down
5 changes: 1 addition & 4 deletions nomt/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ use crate::{
page_cache::PageDiff,
};
use meta::Meta;
use nomt_core::{
page_id::PageId,
trie::KeyPath,
};
use nomt_core::{page_id::PageId, trie::KeyPath};
use parking_lot::Mutex;
use std::{
fs::{File, OpenOptions},
Expand Down

0 comments on commit 1ab58d6

Please sign in to comment.