Implement wal replay for add_edge operations
#2425
Conversation
| ) { | ||
| let e_id = e_id.into(); | ||
| self.add_outbound_edge_inner::<i64>(None, src_pos, dst, e_id.with_layer(0), lsn); | ||
| self.add_outbound_edge_inner::<i64>(None, src_pos, dst, e_id.with_layer(0)); |
There was a problem hiding this comment.
the 0 in with_layer should also be STATIC_GRAPH_LAYER_ID
| ) { | ||
| let e_id = e_id.into(); | ||
| self.add_inbound_edge_inner::<i64>(None, dst_pos, src, e_id.with_layer(0), lsn); | ||
| self.add_inbound_edge_inner::<i64>(None, dst_pos, src, e_id.with_layer(0)); |
There was a problem hiding this comment.
the 0 in with_layer should also be STATIC_GRAPH_LAYER_ID
db4-storage/src/pages/mod.rs
Outdated
| // Node types handling for WriteAndMerge is done in db4-disk-storage | ||
| // For NoOpStrategy, disk_storage_enabled() returns false, so this is skipped | ||
| if EXT::disk_storage_enabled() { | ||
| // Node types will be handled by db4-disk-storage's implementation | ||
| } | ||
|
|
There was a problem hiding this comment.
Was some nonsense generated when I tried to use AI to refactor some stuff, removed
| if let Some(graph_dir) = graph_dir { | ||
| // Config saving for WriteAndMerge is handled in db4-disk-storage's implementation | ||
| // This generic code in db4-storage doesn't have access to WriteAndMergeConfig types | ||
| // to avoid circular dependencies. For NoOpStrategy, config saving is not needed. | ||
| if EXT::disk_storage_enabled() { | ||
| // Config will be saved by db4-disk-storage's GraphStore implementation | ||
| } | ||
| } | ||
|
|
|
|
||
| pub(crate) fn load_from(path: impl AsRef<Path>) -> Result<Self, GraphError> { | ||
| let graph = GraphStorage::Unlocked(Arc::new(TemporalGraph::load_from_path(path)?)); | ||
| let config = Config::load_from_dir(path.as_ref()).unwrap_or_else(|_| Config::default()); |
There was a problem hiding this comment.
Probably shouldn't have this default here! If the config can't be read the graph is unlikely to work (segment sizes might be wrong)
|
|
||
| // Replay any pending writes from the WAL. | ||
| let mut write_locked_graph = temporal_graph.write_lock()?; | ||
| wal.replay_to_graph(&mut write_locked_graph)?; |
There was a problem hiding this comment.
We should probably change this such that it doesn't lock the graph unnecessarily when there is nothing to be replayed.
There was a problem hiding this comment.
Added a wal.has_entries method to check if replay is needed.
| impl DurabilityOps for Storage { | ||
| fn transaction_manager(&self) -> &TransactionManager { | ||
| self.graph.mutable().unwrap().transaction_manager.as_ref() | ||
| } | ||
|
|
||
| fn wal(&self) -> &Wal { | ||
| self.graph.mutable().unwrap().wal() | ||
| } | ||
| } |
There was a problem hiding this comment.
These unwraps are not great
| pub trait InheritDurabilityOps: Base {} | ||
|
|
||
| impl<G: InheritDurabilityOps> DurabilityOps for G | ||
| where | ||
| G::Base: DurabilityOps, | ||
| { | ||
| #[inline] | ||
| fn transaction_manager(&self) -> &TransactionManager { | ||
| self.base().transaction_manager() | ||
| } | ||
|
|
||
| #[inline] | ||
| fn wal(&self) -> &Wal { | ||
| self.base().wal() | ||
| } | ||
| } |
There was a problem hiding this comment.
Not sure this inheritance thing is needed here, probably easier to keep this only on the storage itself. You shouldn't need to call these things on a view (and even if you want to, you always have access to the underlying storage).
There was a problem hiding this comment.
DurabilityOps is now implemented only at the GraphStorage and below levels.
Implements full wal logging and replay for
add_edgeoperations.Also introduces new config objects such as
PersistenceConfigfor tuning storage parameters.