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

raftLog: decouple log data structure and flow control #142

Open
pav-kv opened this issue Jan 27, 2024 · 8 comments
Open

raftLog: decouple log data structure and flow control #142

pav-kv opened this issue Jan 27, 2024 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@pav-kv
Copy link
Contributor

pav-kv commented Jan 27, 2024

The raftLog structure currently plays multiple roles. Notably:

  • Provides read/write access to the raft log (both the unstable in-memory part, and the Storage part), and keeps the basic metadata about the log.
  • To some degree, ensures correctness w.r.t. the core raft algorithm.
  • Implements flow control mechanisms for applying the commands from this log.

The flow control role is quite distinct from the other roles, and is less fundamental. It should be decoupled. Once decoupling is done, raftLog can be isolated in its own package and rigorously tested without assumptions about the flow.

Related to #64, though this clean-up has value on its own.

@pav-kv pav-kv added enhancement New feature or request good first issue Good for newcomers labels Jan 27, 2024
@Elbehery
Copy link
Member

@pav-kv can i take this ?

@pav-kv
Copy link
Contributor Author

pav-kv commented Jan 27, 2024

@Elbehery Sure, give it a try. I got to say that good first issue might be an understatement, doing this requires some familiarity with this repo. LMK if I can help figuring things out (I don't know if you're already familiar).

The complexity may vary here depending on how far we go with it. But we can start with simple mechanical decoupling, and revisiting the tests.

Also, let's not merge big changes before the release #89 is done. We can prepare the change in parallel though.

@Elbehery
Copy link
Member

sgtm, please assign me 👍🏽

@pav-kv
Copy link
Contributor Author

pav-kv commented Jan 30, 2024

@Elbehery Feel free to send PRs / contribute and tag things "Part of #142". This is more like an umbrella issue and doesn't need an assignee, IMO. I'm also working on related parts at the moment.

@nizanshami
Copy link

Is this still needed? This looks like a nice task, and I would be happy to work on it.

@ramses
Copy link

ramses commented Oct 27, 2024

Is this still needed? This looks like a nice task, and I would be happy to work on it.

+1

@samuelbartels20
Copy link
Contributor

@pav-kv Please has this issue been resolved. If not, i would like love to work on it

@pav-kv
Copy link
Contributor Author

pav-kv commented Nov 10, 2024

I don't think it's resolved, feel free to contribute / send PRs. Please note that I'm not active on this repository, you might want to coordinate with maintainers/approvers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants