[Raft] Add generic raftstore - #1525
Conversation
2fd1b75 to
4e0c128
Compare
7170586 to
1e2ae99
Compare
7746510 to
ff8895f
Compare
ff8895f to
22043b4
Compare
7f0c1ad to
14951fc
Compare
05a69b1 to
9c2ae41
Compare
aaae8fd to
a81850a
Compare
a81850a to
84969a4
Compare
| ) | ||
|
|
||
| type RaftRepo[R any] interface { | ||
| GetRepo() R |
There was a problem hiding this comment.
Not sure I understand why GetRepo() is needed? This makes the struct return itself. Why not taking the struct directly in the first place?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()
}84969a4 to
e2ba8a2
Compare
mickmis
left a comment
There was a problem hiding this comment.
LGTM modulo comment + revert or implementation of alternative for MemStore
Taking liberty to dismiss review as @barroco is away and I believe all of his comments were adressed.
37118ae to
ac1ae23
Compare
Implements the generic raftstore which processes and applies committed entries.