raft/tracker: visit Progress in stable order #11004
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11004 +/- ##
=========================================
- Coverage 63.9% 63.8% -0.1%
=========================================
Files 401 401
Lines 37558 37572 +14
=========================================
- Hits 24001 23973 -28
- Misses 11926 11970 +44
+ Partials 1631 1629 -2
Continue to review full report at Codecov.
|
| @@ -166,10 +166,34 @@ func (p *ProgressTracker) Committed() uint64 { | |||
| return uint64(p.Voters.CommittedIndex(matchAckIndexer(p.Progress))) | |||
| } | |||
|
|
|||
| // Visit invokes the supplied closure for all tracked progresses. | |||
| func insertionSort(sl []uint64) { | |||
There was a problem hiding this comment.
move this to a common util pkg? no need for two insertionSort.
There was a problem hiding this comment.
I intentionally didn't do that (https://www.youtube.com/watch?v=PAAkCSZUG1c&t=9m28s). Just to double check, are you suggesting I make an insertionsortutil package that only includes this method? Because it won't be able to include much more than that because quorum's dependency tree is quite small and that's a good thing (plus, lots of our existing util-like code actually depends on raft).
|
lgtm |
| // We need to sort the IDs and don't want to allocate since this is hot code. | ||
| // The optimization here mirrors that in `(MajorityConfig).CommittedIndex`, | ||
| // see there for details. | ||
| var sl [7]uint64 |
There was a problem hiding this comment.
I'd say to avoid the double slice, but you're moving this code from somewhere else, so feel free to keep as is.
var ids []uint64
var sl [7]uint64
if n <= len(sl) {
ids = sl[:n]
} else {
ids = make([]uint64, n)
}
There was a problem hiding this comment.
Fixed both up (the one here was even wrong!)
| @@ -166,10 +166,34 @@ func (p *ProgressTracker) Committed() uint64 { | |||
| return uint64(p.Voters.CommittedIndex(matchAckIndexer(p.Progress))) | |||
| } | |||
|
|
|||
| // Visit invokes the supplied closure for all tracked progresses. | |||
| func insertionSort(sl []uint64) { | |||
This is helpful for upcoming testing work which allows datadriven testing of the interaction of multiple nodes. This testing requires determinism to work correctly.
tbg
force-pushed
the
interaction/unused-type
branch
from
5feebe1
to
1b3e082
Compare
|
Thanks for the reviews! |
This is helpful for upcoming testing work which allows datadriven testing
of the interaction of multiple nodes. This testing requires determinism to
work correctly.