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

Add VCL_Shutdown to wait for VCL references to vanish #4078

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nigoroll
Copy link
Member

@nigoroll nigoroll commented Mar 7, 2024

A panic with SLASH/fellow exposed a problem in Varnish-Cache: We shut down stevedores unconditionally, no matter if still used or not. While an argument could be made that stevedores should wait for all object references to be returned before actually shutting down (which SLASH/fellow does, but limited to a fixed number of retries), this would not have helped in this particular case, because a .free_space variable was queried which, by definition, requires no reference on stevedore objects whatsoever.

So this PR adds VCL_Shutdown() to wait for all VCL references to vanish before closing stevedores.

@nigoroll nigoroll marked this pull request as draft March 7, 2024 21:06
@nigoroll
Copy link
Member Author

nigoroll commented Mar 7, 2024

A lot of tests on CCI are timing out, which is interesting because I did not see this locally.

@nigoroll
Copy link
Member Author

bugwash

  • @mbgrydeland your feedback on anything like this would be greatly appreciated.
  • it has been pointed out that whatever wait might be indefinite, depending on the use case, it it remains undecided if this should be something to address within varnish or from the outside (read: SIGKILL)
  • using the warn call should allow the stevedore to replace its function pointers with some which always fail, making the problem at least less likely to be hit

Before shutting down stevedores, we should wait for VCL references to go
away to, indirectly, ensure that all worker threads potentially using
storage functions are done.

See https://gitlab.com/uplex/varnish/slash/-/issues/61
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant