Skip to content

[Raft] Add generic raftstore - #1525

Merged
mickmis merged 7 commits into
interuss:masterfrom
Orbitalize:add_generic_raftstore
Aug 4, 2026
Merged

[Raft] Add generic raftstore#1525
mickmis merged 7 commits into
interuss:masterfrom
Orbitalize:add_generic_raftstore

Conversation

@MariemBaccari

@MariemBaccari MariemBaccari commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Implements the generic raftstore which processes and applies committed entries.

@MariemBaccari
MariemBaccari marked this pull request as ready for review June 15, 2026 08:14
@MariemBaccari
MariemBaccari marked this pull request as draft June 15, 2026 08:17
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch from 2fd1b75 to 4e0c128 Compare June 15, 2026 08:23
@MariemBaccari
MariemBaccari marked this pull request as ready for review June 15, 2026 08:24
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch 5 times, most recently from 7170586 to 1e2ae99 Compare June 16, 2026 08:49
@barroco barroco added the dss-raft Relating to the application-layer consensus implemenation based on raft label Jun 17, 2026
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch 2 times, most recently from 7746510 to ff8895f Compare June 18, 2026 07:38
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch from ff8895f to 22043b4 Compare June 18, 2026 08:01
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch 4 times, most recently from 7f0c1ad to 14951fc Compare June 19, 2026 13:09
@MariemBaccari
MariemBaccari marked this pull request as ready for review July 1, 2026 09:40
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch from 05a69b1 to 9c2ae41 Compare July 6, 2026 11:53
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch 8 times, most recently from aaae8fd to a81850a Compare July 10, 2026 10:55
@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch from a81850a to 84969a4 Compare July 30, 2026 13:40
Comment thread pkg/raftstore/consensus/proposal.go Outdated
Comment thread pkg/raftstore/consensus/proposal.go Outdated
Comment thread pkg/raftstore/store.go
)

type RaftRepo[R any] interface {
GetRepo() R

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand why GetRepo() is needed? This makes the struct return itself. Why not taking the struct directly in the first place?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I attempted to remove it in this commit but at the cost of a not-so-pretty type assertion repo, ok := r.(R). Let me know what you think / if you have a better or more idiomatic alternative.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I now see the issue, indeed the assertion is not nice, your previous solution is better.
Alternatively we could also have Init accept 2 arguments (MemRepo and R) but that's roughly equivalent, so up to you.

Actually it would have been nice to be able to do the following, however that's not legal in Go:

type MemRepo[R any] interface {
	T
	GetSnapshot() ([]byte, error)
	RestoreFromSnapshot([]byte) error
	Checkpoint()
	Restore()
}

Comment thread pkg/raftstore/store.go
Comment thread pkg/raftstore/store.go
Comment thread pkg/raftstore/store.go Outdated
Comment thread pkg/raftstore/store.go

@mickmis mickmis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo comment + revert or implementation of alternative for MemStore

Comment thread pkg/raftstore/consensus/consensus.go Outdated
@mickmis
mickmis dismissed barroco’s stale review August 4, 2026 12:58

Taking liberty to dismiss review as @barroco is away and I believe all of his comments were adressed.

@MariemBaccari
MariemBaccari force-pushed the add_generic_raftstore branch from 37118ae to ac1ae23 Compare August 4, 2026 13:39
@MariemBaccari
MariemBaccari requested a review from mickmis August 4, 2026 13:40
@mickmis
mickmis merged commit 13bb602 into interuss:master Aug 4, 2026
12 checks passed
@mickmis
mickmis deleted the add_generic_raftstore branch August 4, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dss-raft Relating to the application-layer consensus implemenation based on raft

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants