You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Current code in the hook executor seems to implement a timeout using the time.Timer rather than context.WithTimeout. The shortcoming of this, is that any outgoing requests or other activity is blind to whether the goroutine should continue to run. If a timeout occurs, the hook still continues running indefinitely.
The proposal would be to:
Use context.WithTimeout and defer cancel()
select either the response channel or the <-ctx.Done()
Ensure the error is reported regardless of success or failure (as, I believe it already is)
This would ensure that any work being done in a module hook and, especially, outgoing http requests, would be aware of this timeout and stop working as soon as possible, rather than continuing indefinitely when its response will be ignored.
The text was updated successfully, but these errors were encountered:
This is related to #3962. We'll need to first pass the auction context to the module infrastructure.
It would also be beneficial to add additional metrics that allow us to visualize how often different types of auction errors like context timeout, deadline exceeded, etc are occurring which might require further discussion. @bsardo to open another ticket for this so we can nail down the requirements.
bsardo
changed the title
[Placeholder from PMC] Improve the timeout mechanism in the hook executor
Improve the timeout mechanism in the hook executor
Nov 1, 2024
Current code in the hook executor seems to implement a timeout using the time.Timer rather than context.WithTimeout. The shortcoming of this, is that any outgoing requests or other activity is blind to whether the goroutine should continue to run. If a timeout occurs, the hook still continues running indefinitely.
The proposal would be to:
This would ensure that any work being done in a module hook and, especially, outgoing http requests, would be aware of this timeout and stop working as soon as possible, rather than continuing indefinitely when its response will be ignored.
The text was updated successfully, but these errors were encountered: