-
Notifications
You must be signed in to change notification settings - Fork 86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Node] Add retry on DA connection #1054
Conversation
Thanks for the update on the error message, that is greatly needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran the test and it passed; if the consensus is that the retry logic is needed then I'm willing to approve. Once concern is to have a "service available" condition in the orchestration to prevent this problem that causes the need for retry logic... maybe this could be something to add in a next iteration.
Please fix the test workflow to not reference a nonexistent package. |
Fixed, there's some test that still fail like |
Yes, I see the same failure on #1038, let's fix it separately. |
Summary
protocol-units
,networks
,scripts
,util
,cicd
, ormisc
.Sometime during the docker container start, the full node start too early and the DA light node didn't have opened its entry point. It results in an error during the full node start, then a restart of the whole stack.
This scenario can repeat several times until the timing is correct.
This PR add a retry for this connection so that full node start is less sensible to timing issues during the start.
I've changed the error message when the config can't be parsed.
Changelog
Testing
To test, you can start a leader and its follower:
The test execution should have no errors.
Outstanding issues