datafuselabs / openraft

rust raft with improvements
Apache License 2.0
1.35k stars 148 forks source link

semi-async `RaftLogReader::try_get_log_entries` #1155

Open drmingdrmer opened 1 month ago

drmingdrmer commented 1 month ago

openraft/src/core/sm/worker.rs line 166 at r1 (raw file):

        let n_entries = end - since;

        let apply_results = self.state_machine.apply(entries).await?;

Now that this is in the same method, it would make sense to somehow overlap the processing. E.g., change the interface of getting log entries to a (semi-async) iterator (which can do prefetching as it sees fit for on-disk data) and feed this iterator to apply() directly? The result could be another (semi-async) iterator, which you can then use to get rid of applying_entries.

Originally posted by @schreter in https://github.com/datafuselabs/openraft/pull/1154#pullrequestreview-2161901546

Upgrade RaftLogReader::try_get_log_entries() -> Result<Vec<Entry>, StorageError> to return a pollable result, a Stream or an iterator of Future. So that prefetch can be done before the caller consuming log entries.

trait RaftLogReader {
  // Return a Stream:
  async fn try_get_log_entries(&mut self, range) -> impl Stream<Item=Result<Entry, StorageError>>;

  // Or return an iterator of Future
  type Fu: Future<Item=Result<Entry, StorageError>>;
  async fn try_get_log_entries(&mut self, range) -> impl Iterator<Item=Fu>;
}
github-actions[bot] commented 1 month ago

👋 Thanks for opening this issue!

Get help or engage by:

drmingdrmer commented 1 month ago

@schreter Does these two forms of return type look good to you?

Usually I prefer Stream because it is more generic than a iterator of Future, because itself is poll-able, but an iterator might block a task when fetching the next Future item.

schreter commented 1 month ago

Usually I prefer Stream because it is more generic than a iterator of Future

I am open to what to use. Stream is probably more "modern" and allows a non-blocking implementation, so it's most likely preferable, indeed.