Proposal: Remove RaftLogReader Trait Dependency from RaftLogStorage
#1118
Comments
|
👋 Thanks for opening this issue! Get help or engage by:
|
|
Fine for me :-). In our codebase, we made an indirection to support log reading from both sides. This will become obsolete. |
drmingdrmer
added a commit
to drmingdrmer/openraft
that referenced
this issue
Jul 6, 2024
This commit removes the dependency of the `RaftLogReader` trait from the `RaftLogStorage` interface in Openraft. `trait RaftLogStorage<C>: RaftLogReader<C> + ...` becomes `trait RaftLogStorage<C>: ...`. This change reflects the infrequent need for reading log entries directly from storage, as it primarily occurs when applying logs to the state machine. Inside Openraft, logs can now be read by acquiring a reader via `RaftLogStorage.get_log_reader().await.xxx()`, rather than requiring `RaftLogStorage` to implement the `RaftLogReader` trait. Upgrade(non-breaking) tip: Implementations are advised (though it is not mandatory) to remove the `impl RaftLogReader for YourRaftLogStore` from their codebase. - Fix: databendlabs#1118
drmingdrmer
added a commit
that referenced
this issue
Jul 7, 2024
This commit removes the dependency of the `RaftLogReader` trait from the `RaftLogStorage` interface in Openraft. `trait RaftLogStorage<C>: RaftLogReader<C> + ...` becomes `trait RaftLogStorage<C>: ...`. This change reflects the infrequent need for reading log entries directly from storage, as it primarily occurs when applying logs to the state machine. Inside Openraft, logs can now be read by acquiring a reader via `RaftLogStorage.get_log_reader().await.xxx()`, rather than requiring `RaftLogStorage` to implement the `RaftLogReader` trait. Upgrade(non-breaking) tip: Implementations are advised (though it is not mandatory) to remove the `impl RaftLogReader for YourRaftLogStore` from their codebase. - Fix: #1118
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Current Implementation
RaftLogStorage<C>is currently a subtrait ofRaftLogReader<C>:Proposed Change
As our codebase has evolved, the main loop no longer requires reading logs from
RaftLogStorage. Thus, I propose removingRaftLogReaderas a dependency from theRaftLogStoragetrait:Rationale
This change will simplify the trait hierarchy, reduce unnecessary coupling, and better align with our current usage, where log reading in the main loop is not needed.
I appreciate any feedback on this proposal and am ready to discuss further or adjust the approach based on team input.
The text was updated successfully, but these errors were encountered: