Skip to content

Add VCL_Shutdown to wait for VCL references to vanish #4078

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

Open
wants to merge 2 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

@idl0r
Copy link

idl0r commented May 22, 2025

This looks somewhat "trivial". Is there anything blocking this change/feature?

@nigoroll
Copy link
Member Author

nigoroll commented May 22, 2025

When I last worked in this feature, the CCI checks were timing out, so I suspected that something might be fundamentally wrong. I am re-checking now and have rebased to begin with.

@idl0r
Copy link

idl0r commented May 22, 2025

Ah, ok. Thanks!

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
@nigoroll
Copy link
Member Author

Found the trivial mistake, fixed in 44bb699, subsequently squashed

@nigoroll nigoroll marked this pull request as ready for review May 22, 2025 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants