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

Log interface decoupling #82

Open
yossigo opened this issue Jul 12, 2018 · 5 comments
Open

Log interface decoupling #82

yossigo opened this issue Jul 12, 2018 · 5 comments

Comments

@yossigo
Copy link
Contributor

yossigo commented Jul 12, 2018

Here's a major refactoring step to consider.

Currently the server logic and log implementation are quite coupled, to the point where the log implementation in many cases is responsible to trigger operations on the Raft side.

Decoupling would not just clean things up, but also make it possible to replace the log implementation. For example, it may be useful to implement a disk based log that does not require all entries to be in memory at all time and can fetch them from disk on demand.

@liw
Copy link
Contributor

liw commented Jul 13, 2018

log's interface (and perhaps also callbacks) could be "closer" to what server wants. For instance, "get the term of an entry", "get a reference to a range of entries, including their data", "put a reference to a range of entries", etc. That would allow us to remove the circular buffer implementation and users to implement entry caching and referencing according their specific storage (possibly using hash tables, trees, etc., provided by other libraries).

@freeekanayaka
Copy link
Contributor

@liw what you describe is indeed very close to the design of the log store interface exposed by hashicorp's raft implementation (in Go).

I'm porting a raft-based application from Go to C and not requiring all logs to be in memory all the time would be precisely the use case I have at hand.

@willemt
Copy link
Owner

willemt commented Aug 12, 2018

I like the idea of decoupling the log implementation a lot.

I would first like to finish "batchifying" the log entry API (#51). This is because I don't want to specify a log entry API interface which will change immediately once we add batching APIs.

@yossigo
Copy link
Contributor Author

yossigo commented Nov 16, 2018

There is already quite a lot of work in progress on this:
https://github.com/yossigo/raft/blob/03212d48950054993d20480b8b7cab50be7ec4db/include/raft.h#L410

Some observations and ideas moving forward:

  1. The log interface above is already (mostly) capable of batch processing entries. It's not really enough, because:
    a) Also core Raft callbacks need to be adapted.
    b) In the real world batching won't be enough to improve performance, and some form of async work will be needed to avoid the Raft main loop blocking on I/O.
  2. The existing in-memory log implemented by raft_log.c is wrapped and usable as default internal implementation. Callbacks that are Log-related rather than Raft-related have been split from raft_cbs to raft_log_cbs, and are used by the raft_log.c implementation directly to maintain compatibility.
  3. Most but not all invocations of Log -> Raft have been removed, this is still in progress. There is no real reason for them anyway.
  4. This is also a good opportunity to address ownership of raft_entry_t passing around, possibly reference counting and a custom free function.

@yossigo
Copy link
Contributor Author

yossigo commented Dec 6, 2018

@willemt this turned out to be a bigger deal than expected, with API changes leaking elsewhere but I think it's good progress. There's a branch that seems quite stable, tests passing etc.

https://github.com/yossigo/raft/tree/feature/pluggable-log-impl

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

4 participants