-
Notifications
You must be signed in to change notification settings - Fork 211
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
[Merged by Bors] - Fix timing out querying proof in 1:N in presence of a broken Poet #5943
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5943 +/- ##
=======================================
Coverage 80.7% 80.8%
=======================================
Files 288 288
Lines 29894 29894
=======================================
+ Hits 24147 24157 +10
+ Misses 4159 4153 -6
+ Partials 1588 1584 -4 ☔ View full report in Codecov by Sentry. |
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.
Yes I also think the caller of Proof should probably set the timeout, but I don't insist on it.
bors merge |
Build failed: |
bors merge |
Pull request successfully merged into develop. Build succeeded: |
Fix timing out querying proof in 1:N in presence of a broken Poet (#5943)
Motivation
Description
Moved the context with deadline creation before locking the mutex. This way, all callers have the same timeout, instead of:
💡 It's arguable whether the deadline should be defined in the
Proof()
method, or by the caller of it. Placing it in theProof()
method is easier to test, hence I kept it there.Test Plan
Added a test that verifies N concurrent calls to
poetClient.Proof()
all end withinconfig.RequestTimeout +- <100ms error margin>
.TODO