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

semi-async RaftLogReader::try_get_log_entries #1155

Open
drmingdrmer opened this issue Jul 8, 2024 · 3 comments
Open

semi-async RaftLogReader::try_get_log_entries #1155

drmingdrmer opened this issue Jul 8, 2024 · 3 comments

Comments

@drmingdrmer
Copy link
Member

drmingdrmer commented Jul 8, 2024

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 #1154 (review)

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>;
}
Copy link

github-actions bot commented Jul 8, 2024

👋 Thanks for opening this issue!

Get help or engage by:

  • /help : to print help messages.
  • /assignme : to assign this issue to you.

@drmingdrmer
Copy link
Member Author

@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
Copy link
Collaborator

schreter commented Jul 8, 2024

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.

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

No branches or pull requests

2 participants