Process head_chains in descending order of number of peers#8859
Process head_chains in descending order of number of peers#8859mergify[bot] merged 2 commits intosigp:release-v8.1from
Conversation
pawanjay176
added
ready-for-review
|
Some required checks have failed. Could you please take a look @pawanjay176? 🙏 |
mergify
bot
added
waiting-on-author
| .collect::<Vec<_>>(); | ||
| preferred_ids.sort_unstable(); | ||
| // Sort in descending order | ||
| preferred_ids.sort_unstable_by(|a, b| b.cmp(a)); |
There was a problem hiding this comment.
I'm satisfied that this works for the !chain.is_syncing() part too, as the not-syncing chains will have true, which sorts higher than false (syncing).
There was a problem hiding this comment.
Yeah I agree. Based on the comment, I think the intention was always to construct preferred_ids such that the descending sort works, but we sorted ascending by mistake.
|
|
||
| let mut syncing_chains = SmallVec::<[Id; PARALLEL_HEAD_CHAINS]>::new(); | ||
| for (_, _, id) in preferred_ids { | ||
| let chain = self.head_chains.get_mut(&id).expect("known chain"); |
There was a problem hiding this comment.
Spicy little expect here 😱
There was a problem hiding this comment.
Probably fine, could change it to a crit! in some other PR
michaelsproul
added
ready-for-review
Merge Queue StatusRule:
This pull request spent 31 minutes 25 seconds in the queue, including 29 minutes 39 seconds running CI. Required conditions to merge
|
3 participants
Issue Addressed
N/A
Proposed Changes
Another find by @gitToki. Sort the preferred_ids in descending order as originally intended from the comment in the function.