Skip to content
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

Improve the timeout mechanism in the hook executor #3963

Open
scr-oath opened this issue Oct 9, 2024 · 1 comment
Open

Improve the timeout mechanism in the hook executor #3963

scr-oath opened this issue Oct 9, 2024 · 1 comment

Comments

@scr-oath
Copy link

scr-oath commented Oct 9, 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:

  1. Use context.WithTimeout and defer cancel()
  2. select either the response channel or the <-ctx.Done()
  3. 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.

@bsardo
Copy link
Collaborator

bsardo commented Nov 1, 2024

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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Dev
Development

No branches or pull requests

2 participants