fix(state-parts-dump-check): bound process_part by timeout #10215
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ppca
requested review from
akhi3030 and
nikurt
and removed request for
akhi3030
nikurt
approved these changes
Nov 20, 2023
nikurt
reviewed
Nov 20, 2023
|
|
||
| const MAX_RETRIES: u32 = 3; | ||
| const MAX_RETRIES: u32 = 5; |
There was a problem hiding this comment.
Please add a comment why we shouldn't set this number to a very large number, like 1_000_000.
ppca
force-pushed
the
xiangyi/add_timeout_to_dump_monitoring
branch
from
aa16ab2 to
da2e312
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #10215 +/- ##
==========================================
- Coverage 71.88% 71.85% -0.04%
==========================================
Files 707 707
Lines 141788 141796 +8
Branches 141788 141796 +8
==========================================
- Hits 101927 101888 -39
- Misses 35147 35196 +49
+ Partials 4714 4712 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem:
Occasionally, the monitoring will be stuck. What happens is there are a few parts for which the process_part() is called, but never finished, i.e. not retried, not success, not failed.
My guess is that the get_part().await somehow is stuck waiting forever, and since there was no timeout bound for the process_part(), the tokio task that runs process_part could be hung forever, thus the monitoring app is stuck.
I tried making the task for part_id = 100 sleep for 1000000 seconds, and the program finishes all other parts except this one and hangs. This is similar to what I see on monitoring node.
Solution:
bound process_part by a timeout, and initiate retry if timeout passed. Thus no matter for which reason the task hangs, it will retry itself.
Note, the timeout is set to 10 mins because the timeout starts counting once the task is spawn, and counts real time instead of cpu time for the task, i.e. if this task is not running, time still elapse. So we need to make sure within timeout * MAX_RETRIES, we are able to finish processing all parts.