Introduce Reducer Trait #40
Conversation
There was a problem hiding this comment.
Overall this looks solid. In order to land this make sure that the following commands run with no error and no compiler warnings (like unused imports):
just build
just unit-test
just test-end-to-end-local-net
just test-end-to-end-local
just package-sqlsync-worker
I'll also figure out how to get your PR's running in github actions.
| use crate::error::Result; | ||
| use crate::reducer::Reducer; | ||
| use crate::replication::{ReplicationDestination, ReplicationError, ReplicationSource}; | ||
| use crate::reducer::{Reducer, WasmReducer}; |
There was a problem hiding this comment.
unused import WasmReducer
| storage: J, | ||
| timeline_factory: J::Factory, | ||
| reducer_wasm_bytes: &[u8], | ||
| reducer: R, // reducer_wasm_bytes: &[u8], |
|
|
||
| /// CoordinatorDocument knows how to replicate it's storage journal | ||
| impl<J: Journal + ReplicationSource> ReplicationSource for CoordinatorDocument<J> { | ||
| impl<J: Journal + ReplicationSource, R: Reducer> ReplicationSource |
There was a problem hiding this comment.
type constraint not needed here
|
|
||
| use serde::{Deserialize, Serialize}; | ||
|
|
||
| /// Log Sequence Number |
There was a problem hiding this comment.
I think this is the only actual change in this file? Can you revert the other changes since I think they are just due to our rustfmt settings differing. I'll figure out how to enforce consistent rustfmt settings for the whole repo to prevent this in the future.
| pub trait Reducer { | ||
| fn apply(&mut self, tx: &mut Transaction, mutation: &[u8]) -> Result<()>; | ||
| } | ||
|
|
||
| impl Reducer for WasmReducer { | ||
| fn apply(&mut self, tx: &mut Transaction, mutation: &[u8]) -> Result<()> { | ||
| WasmReducer::apply(self, tx, mutation) | ||
| } | ||
| } |
There was a problem hiding this comment.
Pretty stoked how clean this ended up being. If you have time, I would prefer to just move the WasmReducer apply impl into this impl, which probably means you'll need to make the reducer generic in local.rs (which looks like it's just a couple lines of changes a'la coordinator). No worries if you leave this, I can do it later.
There was a problem hiding this comment.
Missing WasmReducer on line 116 I think.
error[E0107]: struct takes 2 generic arguments but 1 generic argument was supplied
--> lib/sqlsync/examples/end-to-end-local-net.rs:116:20
|
116 | doc: Arc<Mutex<CoordinatorDocument<MemoryJournal>>>,
| ^^^^^^^^^^^^^^^^^^^ ------------- supplied 1 generic argument
| |
| expected 2 generic arguments
|
note: struct defined here, with 2 generic parameters: `J`, `R`
--> /home/carl/dev/sqlsync/lib/sqlsync/src/coordinator.rs:25:12
|
25 | pub struct CoordinatorDocument<J: Journal, R> {
| ^^^^^^^^^^^^^^^^^^^ - -
help: add missing generic argument
|
116 | doc: Arc<Mutex<CoordinatorDocument<MemoryJournal, R>>>,
| +++
```
|
I think if you rebase this on main it should prompt me to run tests on your PRs. |
|
I'm going to clean this PR up and merge it now - just pinging incase you are also doing so. |
Thanks! Sorry for the lint issues. My LSP was falling over yesterday. |
3 participants
Resolves #39
Decided to have a go at implementing the reducer trait so that custom server dead letters and such can be inserted based on mutation content and the user's specific business logic. Seems to be working fine in my server but haven't tested exhaustively.
Then users can use it like so
Most of the lines are changed because of auto formatting.