-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Allow development of custom stores #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a326642
ff49ac6
396adc4
e6cb821
b9c658e
2516683
6507f71
ba13393
53ddfe2
dc525a1
7f271a7
4d88603
5046d00
cc9998c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| pub mod redb; | ||
|
|
||
| use prost::Message; | ||
| use std::sync::Arc; | ||
| use tokio::sync::Mutex; | ||
|
|
||
| use crate::{Block, ChainPoint, Error}; | ||
|
|
||
| pub type WorkerId = String; | ||
| pub type LogSeq = u64; | ||
|
|
||
| #[derive(Message)] | ||
| pub struct LogEntry { | ||
| #[prost(bytes, tag = "1")] | ||
| pub next_block: Vec<u8>, | ||
| #[prost(bytes, repeated, tag = "2")] | ||
| pub undo_blocks: Vec<Vec<u8>>, | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| pub trait AtomicUpdateTrait { | ||
| async fn update_worker_cursor(&mut self, id: &str) -> Result<(), super::Error>; | ||
| async fn commit(&mut self) -> Result<(), super::Error>; | ||
| } | ||
|
Comment on lines
+20
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix non-Send future hazard from locking tokio::Mutex across await Custom dispatch holds a tokio::MutexGuard across .await, but #[async_trait] defaults to Send futures, leading to "future cannot be sent between threads safely". Make the traits/impls non-Send or refactor to avoid awaiting while holding the guard. Apply this to relax futures: -#[async_trait::async_trait]
+#[async_trait::async_trait(?Send)]
pub trait AtomicUpdateTrait {-#[async_trait::async_trait]
+#[async_trait::async_trait(?Send)]
impl AtomicUpdateTrait for AtomicUpdate {-#[async_trait::async_trait]
+#[async_trait::async_trait(?Send)]
pub trait StoreTrait {-#[async_trait::async_trait]
+#[async_trait::async_trait(?Send)]
impl StoreTrait for Store {Alternative (larger refactor): avoid outer Arc<Mutex> and require implementers to handle interior mutability, so no guard is held across .await. Also applies to: 32-46, 54-65, 67-110 |
||
|
|
||
| #[allow(clippy::large_enum_variant)] | ||
| pub enum AtomicUpdate { | ||
| Redb(redb::AtomicUpdate), | ||
| Custom(Arc<Mutex<dyn AtomicUpdateTrait + Send + Sync>>), | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl AtomicUpdateTrait for AtomicUpdate { | ||
| async fn update_worker_cursor(&mut self, id: &str) -> Result<(), super::Error> { | ||
| match self { | ||
| AtomicUpdate::Redb(au) => au.update_worker_cursor(id).await, | ||
| AtomicUpdate::Custom(au) => au.lock().await.update_worker_cursor(id).await, | ||
| } | ||
| } | ||
| async fn commit(&mut self) -> Result<(), super::Error> { | ||
| match self { | ||
| AtomicUpdate::Redb(au) => au.commit().await, | ||
| AtomicUpdate::Custom(au) => au.lock().await.commit().await, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Clone)] | ||
| pub enum Store { | ||
| Redb(redb::Store), | ||
| Custom(Arc<Mutex<dyn StoreTrait + Send + Sync>>), | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| pub trait StoreTrait { | ||
| async fn find_chain_point(&self, seq: LogSeq) -> Result<Option<ChainPoint>, Error>; | ||
| async fn write_ahead( | ||
| &mut self, | ||
| undo_blocks: &[Block], | ||
| next_block: &Block, | ||
| ) -> Result<LogSeq, Error>; | ||
| async fn get_worker_cursor(&self, id: &str) -> Result<Option<LogSeq>, super::Error>; | ||
| async fn start_atomic_update(&self, log_seq: LogSeq) -> Result<AtomicUpdate, super::Error>; | ||
| async fn handle_reset(&self, point: ChainPoint) -> Result<Vec<Block>, super::Error>; | ||
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl StoreTrait for Store { | ||
| async fn find_chain_point(&self, seq: LogSeq) -> Result<Option<ChainPoint>, Error> { | ||
| match self { | ||
| Store::Redb(store) => store.find_chain_point(seq).await, | ||
| Store::Custom(store) => store.lock().await.find_chain_point(seq).await, | ||
| } | ||
| } | ||
| async fn write_ahead( | ||
| &mut self, | ||
| undo_blocks: &[Block], | ||
| next_block: &Block, | ||
| ) -> Result<LogSeq, Error> { | ||
| match self { | ||
| Store::Redb(store) => store.write_ahead(undo_blocks, next_block).await, | ||
| Store::Custom(store) => { | ||
| store | ||
| .lock() | ||
| .await | ||
| .write_ahead(undo_blocks, next_block) | ||
| .await | ||
| } | ||
| } | ||
| } | ||
| async fn get_worker_cursor(&self, id: &str) -> Result<Option<LogSeq>, super::Error> { | ||
| match self { | ||
| Store::Redb(store) => store.get_worker_cursor(id).await, | ||
| Store::Custom(store) => store.lock().await.get_worker_cursor(id).await, | ||
| } | ||
| } | ||
| async fn start_atomic_update(&self, log_seq: LogSeq) -> Result<AtomicUpdate, super::Error> { | ||
| match self { | ||
| Store::Redb(store) => store.start_atomic_update(log_seq).await, | ||
| Store::Custom(store) => store.lock().await.start_atomic_update(log_seq).await, | ||
| } | ||
| } | ||
|
|
||
| async fn handle_reset(&self, point: ChainPoint) -> Result<Vec<Block>, super::Error> { | ||
| match self { | ||
| Store::Redb(store) => store.handle_reset(point).await, | ||
| Store::Custom(store) => store.lock().await.handle_reset(point).await, | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,21 +1,12 @@ | ||||||||||||||||||||||||||||||||
| use itertools::Itertools; | ||||||||||||||||||||||||||||||||
| use prost::Message; | ||||||||||||||||||||||||||||||||
| use redb::{ReadableTable as _, TableDefinition, WriteTransaction}; | ||||||||||||||||||||||||||||||||
| use std::{collections::VecDeque, path::Path, sync::Arc}; | ||||||||||||||||||||||||||||||||
| use tracing::warn; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| use crate::{Block, ChainPoint, Error}; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pub type WorkerId = String; | ||||||||||||||||||||||||||||||||
| pub type LogSeq = u64; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| #[derive(Message)] | ||||||||||||||||||||||||||||||||
| pub struct LogEntry { | ||||||||||||||||||||||||||||||||
| #[prost(bytes, tag = "1")] | ||||||||||||||||||||||||||||||||
| pub next_block: Vec<u8>, | ||||||||||||||||||||||||||||||||
| #[prost(bytes, repeated, tag = "2")] | ||||||||||||||||||||||||||||||||
| pub undo_blocks: Vec<Vec<u8>>, | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| use super::StoreTrait; | ||||||||||||||||||||||||||||||||
| pub use super::{AtomicUpdateTrait, LogEntry, LogSeq, WorkerId}; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| impl redb::Value for LogEntry { | ||||||||||||||||||||||||||||||||
| type SelfType<'a> | ||||||||||||||||||||||||||||||||
|
|
@@ -58,20 +49,40 @@ const WAL: TableDefinition<LogSeq, LogEntry> = TableDefinition::new("wal"); | |||||||||||||||||||||||||||||||
| const DEFAULT_CACHE_SIZE_MB: usize = 50; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pub struct AtomicUpdate { | ||||||||||||||||||||||||||||||||
| wx: WriteTransaction, | ||||||||||||||||||||||||||||||||
| wx: Option<WriteTransaction>, | ||||||||||||||||||||||||||||||||
| log_seq: LogSeq, | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| impl AtomicUpdate { | ||||||||||||||||||||||||||||||||
| pub fn update_worker_cursor(&mut self, id: &str) -> Result<(), super::Error> { | ||||||||||||||||||||||||||||||||
| let mut table = self.wx.open_table(CURSORS)?; | ||||||||||||||||||||||||||||||||
| pub fn new(wx: WriteTransaction, log_seq: LogSeq) -> Self { | ||||||||||||||||||||||||||||||||
| Self { | ||||||||||||||||||||||||||||||||
| wx: Some(wx), | ||||||||||||||||||||||||||||||||
| log_seq, | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| #[async_trait::async_trait] | ||||||||||||||||||||||||||||||||
| impl AtomicUpdateTrait for AtomicUpdate { | ||||||||||||||||||||||||||||||||
| async fn update_worker_cursor(&mut self, id: &str) -> Result<(), super::Error> { | ||||||||||||||||||||||||||||||||
| let Some(wx) = self.wx.as_mut() else { | ||||||||||||||||||||||||||||||||
| return Err(super::Error::Store( | ||||||||||||||||||||||||||||||||
| "Transaction already commited".to_string(), | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo: "commited" → "committed". The error messages contain a spelling error. Apply this diff: return Err(super::Error::Store(
- "Transaction already commited".to_string(),
+ "Transaction already committed".to_string(),
));Also applies to: 82-82 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| )); | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
Comment on lines
+66
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: fix typo in error message ("committed") - return Err(super::Error::Store(
- "Transaction already commited".to_string(),
- ));
+ return Err(super::Error::Store(
+ "Transaction already committed".to_string(),
+ ));- return Err(super::Error::Store(
- "Transaction already commited".to_string(),
- ));
+ return Err(super::Error::Store(
+ "Transaction already committed".to_string(),
+ ));Also applies to: 80-85 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| let mut table = wx.open_table(CURSORS)?; | ||||||||||||||||||||||||||||||||
| table.insert(id.to_owned(), self.log_seq)?; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pub fn commit(self) -> Result<(), super::Error> { | ||||||||||||||||||||||||||||||||
| self.wx.commit()?; | ||||||||||||||||||||||||||||||||
| async fn commit(&mut self) -> Result<(), super::Error> { | ||||||||||||||||||||||||||||||||
| let Some(wx) = self.wx.take() else { | ||||||||||||||||||||||||||||||||
| return Err(super::Error::Store( | ||||||||||||||||||||||||||||||||
| "Transaction already commited".to_string(), | ||||||||||||||||||||||||||||||||
| )); | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| wx.commit()?; | ||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
@@ -165,15 +176,18 @@ impl Store { | |||||||||||||||||||||||||||||||
| let entry = table.get(seq)?; | ||||||||||||||||||||||||||||||||
| Ok(entry.map(|x| x.value())) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pub fn find_chain_point(&self, seq: LogSeq) -> Result<Option<ChainPoint>, Error> { | ||||||||||||||||||||||||||||||||
| #[async_trait::async_trait] | ||||||||||||||||||||||||||||||||
| impl StoreTrait for Store { | ||||||||||||||||||||||||||||||||
| async fn find_chain_point(&self, seq: LogSeq) -> Result<Option<ChainPoint>, Error> { | ||||||||||||||||||||||||||||||||
| let entry = self.get_entry(seq)?; | ||||||||||||||||||||||||||||||||
| let block = Block::from_bytes(&entry.unwrap().next_block); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Ok(Some(block.chain_point())) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+183
to
188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid panic: handle missing WAL entry gracefully in find_chain_point. entry.unwrap() will panic if the sequence isn’t found. Return Ok(None) instead. Apply this diff: - async fn find_chain_point(&self, seq: LogSeq) -> Result<Option<ChainPoint>, Error> {
- let entry = self.get_entry(seq)?;
- let block = Block::from_bytes(&entry.unwrap().next_block);
-
- Ok(Some(block.chain_point()))
- }
+ async fn find_chain_point(&self, seq: LogSeq) -> Result<Option<ChainPoint>, Error> {
+ match self.get_entry(seq)? {
+ Some(entry) => {
+ let block = Block::from_bytes(&entry.next_block);
+ Ok(Some(block.chain_point()))
+ }
+ None => Ok(None),
+ }
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pub fn write_ahead( | ||||||||||||||||||||||||||||||||
| async fn write_ahead( | ||||||||||||||||||||||||||||||||
| &mut self, | ||||||||||||||||||||||||||||||||
| undo_blocks: &[Block], | ||||||||||||||||||||||||||||||||
| next_block: &Block, | ||||||||||||||||||||||||||||||||
|
|
@@ -196,7 +210,7 @@ impl Store { | |||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // TODO: see if loading in batch is worth it | ||||||||||||||||||||||||||||||||
| pub fn get_worker_cursor(&self, id: &str) -> Result<Option<LogSeq>, super::Error> { | ||||||||||||||||||||||||||||||||
| async fn get_worker_cursor(&self, id: &str) -> Result<Option<LogSeq>, super::Error> { | ||||||||||||||||||||||||||||||||
| let rx = self.db.begin_read()?; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| let table = match rx.open_table(CURSORS) { | ||||||||||||||||||||||||||||||||
|
|
@@ -209,30 +223,15 @@ impl Store { | |||||||||||||||||||||||||||||||
| Ok(cursor.map(|x| x.value())) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| pub fn start_atomic_update(&self, log_seq: LogSeq) -> Result<AtomicUpdate, super::Error> { | ||||||||||||||||||||||||||||||||
| async fn start_atomic_update( | ||||||||||||||||||||||||||||||||
| &self, | ||||||||||||||||||||||||||||||||
| log_seq: LogSeq, | ||||||||||||||||||||||||||||||||
| ) -> Result<super::AtomicUpdate, super::Error> { | ||||||||||||||||||||||||||||||||
| let wx = self.db.begin_write()?; | ||||||||||||||||||||||||||||||||
| Ok(AtomicUpdate { wx, log_seq }) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // TODO: I don't think we need this since we're going to load each cursor as | ||||||||||||||||||||||||||||||||
| // part of the loaded worker | ||||||||||||||||||||||||||||||||
| pub fn lowest_cursor(&self) -> Result<Option<LogSeq>, super::Error> { | ||||||||||||||||||||||||||||||||
| let rx = self.db.begin_read()?; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| let table = rx.open_table(CURSORS)?; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| let cursors: Vec<_> = table | ||||||||||||||||||||||||||||||||
| .iter()? | ||||||||||||||||||||||||||||||||
| .map_ok(|(_, value)| value.value()) | ||||||||||||||||||||||||||||||||
| .try_collect()?; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| let lowest = cursors.iter().fold(None, |all, item| all.min(Some(*item))); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Ok(lowest) | ||||||||||||||||||||||||||||||||
| Ok(super::AtomicUpdate::Redb(AtomicUpdate::new(wx, log_seq))) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// Return list of blocks to undo after receiving a reset response from chainsync. | ||||||||||||||||||||||||||||||||
| pub fn handle_reset(&self, point: ChainPoint) -> Result<Vec<Block>, super::Error> { | ||||||||||||||||||||||||||||||||
| async fn handle_reset(&self, point: ChainPoint) -> Result<Vec<Block>, super::Error> { | ||||||||||||||||||||||||||||||||
| let rx = self.db.begin_read()?; | ||||||||||||||||||||||||||||||||
| let table = rx.open_table(WAL)?; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error handling degradation: String loses type information.
Changing
Error::StorefromBox<redb::Error>toStringis a breaking change that significantly degrades error handling:redberror variantsConsider preserving structured error information while still supporting custom stores. For example, introduce an enum that can hold both
redberrors and a generic string variant for custom store errors:#[derive(Error, Debug)] pub enum Error { // ... other variants - #[error("store error {0}")] - Store(String), + #[error("store error {0}")] + Store(#[from] StoreError), +} + +#[derive(Error, Debug)] +pub enum StoreError { + #[error("redb error: {0}")] + Redb(#[from] redb::Error), + + #[error("redb database error: {0}")] + RedbDatabase(#[from] redb::DatabaseError), + + #[error("redb transaction error: {0}")] + RedbTransaction(#[from] redb::TransactionError), + + #[error("redb table error: {0}")] + RedbTable(#[from] redb::TableError), + + #[error("redb commit error: {0}")] + RedbCommit(#[from] redb::CommitError), + + #[error("redb storage error: {0}")] + RedbStorage(#[from] redb::StorageError), + + #[error("custom store error: {0}")] + Custom(String), }This approach maintains backward compatibility for error inspection while supporting custom store implementations.
Also applies to: 87-121
🤖 Prompt for AI Agents