Skip to content
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

Change: remove N, LS from Raft<C, N, LS, _> #940

Closed
wants to merge 1 commit into from

Conversation

drmingdrmer
Copy link
Member

@drmingdrmer drmingdrmer commented Nov 20, 2023

Changelog

Change: remove N, LS from Raft<C, N, LS, _>
  • Raft<C, ..>: is a control handle of RaftCore and it does not directly
    rely on N: RaftNetworkFactory and LS: RaftLogStorage.
    Thus these two type should not be part of Raft.

    In this commit, we remove N, LS from Raft<C, N, LS, SM>,
    RaftInner<C, N, LS> and RaftMsg<C, N, LS>.
    Type N, LS now is only used by Raft::new() which needs these two
    types to create RaftCore.

  • Raft::external_request(): Another change is the signature of the
    Fn passed to Raft::external_request() changes from
    FnOnce(&RaftState, &mut LS, &mut N) to FnOnce(&RaftState).

  • Fix: the FnOnce passed to Raft::external_request() should always
    be Send, unoptionally. Because it is used to send from Raft to
    RaftCore.

  • Fix: RaftMsg::ExternalRequest: should not rely on RaftLogStorage and RaftNetworkFactory #939


This change is Reviewable

- `Raft<C, ..>`: is a control handle of `RaftCore` and it does not directly
  rely on `N: RaftNetworkFactory`, `LS: RaftLogStorage` and
  `SM: RaftStateMachine`.
  Thus these types should not be part of `Raft`.

  In this commit, we remove `N, LS, SM` from `Raft<C, N, LS, SM>`,
  `RaftInner<C, N, LS>` and `RaftMsg<C, N, LS>`.
  Type `N, LS, SM` now are only used by `Raft::new()` which needs these
  types to create `RaftCore`.

- `Raft::external_request()`: Another change is the signature of the
  `Fn` passed to `Raft::external_request()` changes from
  `FnOnce(&RaftState, &mut LS, &mut N)` to `FnOnce(&RaftState)`.

- Fix: the `FnOnce` passed to `Raft::external_request()` should always
  be `Send`, unoptionally. Because it is used to send from `Raft` to
  `RaftCore`.

- Fix: databendlabs#939
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RaftMsg::ExternalRequest: should not rely on RaftLogStorage and RaftNetworkFactory
1 participant