[Merged by Bors] - activation: panic if post service exits unexpectedly #5300
Conversation
fasmat
force-pushed
the
5267-panic-on-post-service-exit
branch
from
4b21309 to
882bf8a
Compare
|
@poszu the new version of post-rs seems to not compile on windows and shows unusual errors on macOS (it also complains that it was built for a newer macOS than it is executed on). |
fasmat
changed the title
|
The new version of post-rs also seems to validate |
This is the mainnet default. You can overwrite it with |
Weird, it builds on Windows nicely in post-rs CI. Regarding Mac - it was previously built with Mac SDK 12.3 but is now built with the default SDK (no overwrite). Building with an older SDK was a workaround for a bug in RadnomX lib (causing crashes on Mac) that is now fixed. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #5300 +/- ##
=========================================
- Coverage 77.5% 77.4% -0.1%
=========================================
Files 253 253
Lines 29632 29659 +27
=========================================
+ Hits 22966 22979 +13
- Misses 5204 5215 +11
- Partials 1462 1465 +3 ☔ View full report in Codecov by Sentry. |
|
bors merge |
|
👎 Rejected by too few approved reviews |
| } | ||
| eg.Wait() | ||
| ps.logger.Fatal("post service exited", zap.Error(err)) |
There was a problem hiding this comment.
i think it makes more sense to raise Fatal in node when error will bubble up. there are several examples of this, for example peersync process
There was a problem hiding this comment.
You mean just returning an error here and handle the error with log.Fatal in node.go?
The problem is that the error here happens asynchronously and doesn't bubble up at the moment. Returning it here would do nothing (besides preventing the internal errgroup from accepting new go routines).
|
bors merge |
## Motivation Closes #5267 merge after spacemeshos/post#249 ## Changes - PoST supervisor was updated to add `--max-retries` as cmdline parameter to Post service - Number of retries can be configured using the `PostSupervisorConfig` (not exposed via node config or cmdline parameters) and defaults to 10. This gives the node about 50 seconds to be ready for the post service to connect after starting it ## Test Plan - Updated existing tests to match new behavior - Add test that checks if incorrect setup that causes the post service to be unable to connect to the node causes `zap.Fatal` to be called ## TODO <!-- This section should be removed when all items are complete --> - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed - [x] Update [changelog](../CHANGELOG.md) as needed
|
Pull request successfully merged into develop. Build succeeded: |
spacemesh-bors
bot
changed the title
Motivation
Closes #5267 merge after spacemeshos/post#249
Changes
--max-retriesas cmdline parameter to Post servicePostSupervisorConfig(not exposed via node config or cmdline parameters) and defaults to 10. This gives the node about 50 seconds to be ready for the post service to connect after starting itTest Plan
zap.Fatalto be calledTODO