Build: handle Celery timeout events #9387
Closed
Conversation
This file contains hidden or 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
Move exception messages together with other definitions.
We were accessing to `self.task.data` which does not exist. `self.task` is not the same instance as the one it's running, so we don't have `.data` on it. However, we can access the arguments passed when called. So, we are logging that information which is what we have. This commit solves a problem reported on Sentry everytime this happens.
This was never called. However, after fixing the `on_timeout` issue, we need the default behavior here because SIGTERM is used on `TimeLimitExceeded` to _hard_ kill the task and we need that.
Once Celery triggers `SoftTimeLimitExceeded` we immediately set the build error to TIMEOUT because there is high probabilities this task gets killed. So, we take advantage of this event to update the build status properly.
When `SoftTimeLimitExceeded` is reached, we revoke the task and mark it as "Cancelled due timeout" to communicate this clearly to the user.
|
Note that we are not hitting this too frequently because we have an pretty high timeout due to this: readthedocs.org/readthedocs/projects/tasks/utils.py Lines 95 to 100 in 634f85b |
|
I'm closing this PR for now. We can come back in the future if we consider. The idea is good, but I wasn't able to properly QA on production to be sure it works as I expect. |
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.
Solve the problem reported by Sentry when the Celery task times out: https://sentry.io/organizations/read-the-docs/issues/3038754375/
Note to myself: the latest commit tries to handle soft timeout cancelled properly. It's close but it needs a little more of QA because for some reason
SIGUSR1is triggered/catched as well.