Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2887 +/- ##
==========================================
- Coverage 57.22% 57.20% -0.03%
==========================================
Files 2093 2093
Lines 171771 171755 -16
==========================================
- Hits 98294 98249 -45
- Misses 64701 64724 +23
- Partials 8776 8782 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
masih
left a comment
There was a problem hiding this comment.
This is a bigger change than I was expecting: it fixes the bug discussed in Slack, and fundamentally changes the behaviour of the node. In that, after this change the process errors out instead of softly restarting the reactor in-process.
This is clearly simpler, but it means that without an external process supervisor there is no "restart" after self-remediation. The process simply dies.
I probably would have broken this into two PRs, just for the sake of release notes: one to fix the clear bug regarding restart trigger during cooldown, and one to remove it. At the very least this needs a mention in the next release notes for 6.3 patch release. Cc @philipsu522
No blockers.
0fb652c to
9a2738b
Compare
masih
left a comment
There was a problem hiding this comment.
No blockers. Left couple of comments that are worth addressing, either here or in follow up PRs.
Co-authored-by: Masih H. Derkani <m@derkani.org>
…i-chain into gprusak-cooldown
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-2887-to-release/v6.3
git worktree add --checkout .worktree/backport-2887-to-release/v6.3 backport-2887-to-release/v6.3
cd .worktree/backport-2887-to-release/v6.3
git reset --hard HEAD^
git cherry-pick -x 8fc1d5b9f36d09cf5e991687f8bd7bda4f04c5bc
git push --force-with-lease |
There was a semantic mismatch between blocksync sending at most 1 message to the restartCh, and WaitForQuitSignals which ignored messages until cooldown has passed. I think that cooldown in blocksync logic was exactly trying to avoid sending messages when WaitForQuitSignals could have ignored them, which is a very fragile way of doing things. This PR removes cooldown from WaitForQuitSignals, simply delegating the cooldown logic to blocksync and makes the signal nonblocking and idempotent.