-
Notifications
You must be signed in to change notification settings - Fork 381
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 VDI_EVENT_SICK and use it for admin health changes #4196
Conversation
one note here: because the events were only sent for VCL lifetime steps, I think we were guaranteed to be running without in-flight requests (for that VCL). This changes the contract a bit. For context, in the |
There is a relevant change that we extend the facility for director events, but the way I read your understanding of what was guaranteed, I think it might be incomplete: VDI temperature events were already generated outside VCL lifetime steps for dynamic backends here and here. At least the cold event happens outside the
I am not sure which PRIV struct you are referring to, but it sounds unrelated? |
That would be the PRIV_VCL for VMODs subscribing to events. However this event is not directed (pun intended) towards the VMOD itself so it does not violate the mutability assumption you made in the VMOD event callback. |
Ah, thank you both, I did confuse the two events |
Yes, the director |
slightly edited version of Dridis suggestion varnishcache#4196 (comment)
1a42779
to
cd63ef7
Compare
bugwash: ok |
Pondering solutions for varnishcache#4183 it became apparent that we need a way to notify directors of changes to the admin_health without crossing the VDI/VBE layering. Besides adding another director callback, one option is to add an event to the existing event facility, which so far was only used for VCL-related events. We now add VDI_EVENT_SICK as a director-specific event, which is cheap and also helps us maintain clean layering.
Using the new event, we can now selectively notify interested backend types (so far: VBE only) when the admin health changes. This fixes the layer violation introduced with e46f972, where a director private pointer was used with a VBE specific function. Fixes varnishcache#4183
cd63ef7
to
e3d1e29
Compare
Alternative implementation to #4186 as suggested by dridi and phk:
This patch adds an event to the existing event facility, which so far was only used for VCL-related events. We now add
VDI_EVENT_SICK
as a director-specific event, which is cheap and also helps us maintain clean layering.The new event is posted when the admin health changes and used by VBE to cancel all waiting connection requests.