refactor: no more C::Responder = OneshotResponder for change_membership()

[drmingdrmer]

Changelog

refactor: no more C::Responder = OneshotResponder for change_membership()

Before this commit, calling change_membership() required the
RaftTypeConfig::Responder to be configured with OneshotResponder.
This unnecessarily limited applications from using alternative
Responder types.

In this commit, we've removed this restriction from the
change_membership() method, allowing applications to use any responder
type they prefer. Internally, we now store different types of responders
for change_membership() and client_write() operations in an enum.

chore: add comment

• Improvement

---

This change is 

[schreter]

I'm not particularly thrilled at having an enum with the two responders, but not sure how to do it in a better way w/o yet another memory allocation.

We use our own InplaceBox<dyn Trait, SIZE> in our project to solve similar problem by storing the dynamic object in-place (limited by SIZE), but the stuff is not on crates.io (yet). Would it be potentially interesting for you?

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @drmingdrmer)

---

openraft/src/raft/api/management.rs line 26 at r1 (raw file):

/// Provides management APIs for the Raft system.
///
/// This module contains methods for managing the Raft cluster, including

Same here.

---

openraft/src/raft/api/app.rs line 26 at r1 (raw file):

/// Provides application-facing APIs for interacting with the Raft system.
///
/// This module contains methods for client operations such as linearizable reads

This is not a module doc comment. Should it be maybe written with //!?

[drmingdrmer]

InplaceBox sounds great. I think the memory layout of InplaceBox would look like the following. I assume it requires an additional usize to store the vtable?

struct InplaceBox<T, const SIZE: usize> {
    storage: [MaybeUninit<u8>; SIZE],
    vtable: usize;
}

Reviewable status: 8 of 10 files reviewed, all discussions resolved (waiting on @schreter)

---

openraft/src/raft/api/app.rs line 26 at r1 (raw file):

Previously, schreter wrote…

This is not a module doc comment. Should it be maybe written with //!?

Hmm. AI refined these doc for me. I think 'module' here means the following struct but not a mod in rust. Replaced module with struct.

[schreter]

Yes, something similar:

pub struct InplaceBox<T: ?Sized, const SIZE: usize> {
    object_space: MaybeUninit<[u8; SIZE]>,
    vtable: AssertUnwindSafe<<T as Pointee>::Metadata>,
    _phantom: PhantomData<T>,
}

The vtable can't be usize, since otherwise miri can't track pointer provenance properly (integer-to-pointer cast). So we use pointer metadata instead. We also have several static assertions that check the alignment, size, etc. when constructing it via fn InplaceBox::new<'a, U: Sized + Unsize<T> + 'a>(value: U) -> Self. Unfortunately, our impl requires a few nightly features (but almost all of it is doable also without).

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)

---

openraft/src/raft/api/app.rs line 26 at r1 (raw file):

Previously, drmingdrmer (张炎泼) wrote…

Hmm. AI refined these doc for me. I think 'module' here means the following struct but not a mod in rust. Replaced module with struct.

AI, hm... The I in AI is not yet there, hehe :-).

[drmingdrmer]

Interesting. TIL :)

I have a question: when storing a T: Trait as InplaceBox<dyn Trait, _>, the value of T should be copied to object_space. How to make sure the alignment of object_space the same as T? The alignment of [u8; _] is 1 AFAIK.

Reviewed 1 of 2 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)

[schreter]

The alignment of Metadata is 8B, since it is a pointer (thus the entire object is 8B aligned). We statically assert that alignment of a concrete T passed to new() is less than or equal to the alignment of Metadata. Alternatively, using u64 instead of u8 and smaller SIZE also solves the issue.

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)

[drmingdrmer]

Ah, That's a genius way!

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @drmingdrmer)