not respecting implicit parameter dependencies triggers panics
This issue is to track https://varnish-cache.org/lists/pipermail/varnish-dev/2018-January/009208.html
Workspace-related parameters need to be adjusted such that included structures can be accommodated, otherwise panics can happen. One such case is vsl_buffer, if set close to *_workspace, varnish will panic.
We do not want the parameters to be "4kb + overhead" because that would be suboptimal for VM reasons.
We should probably set a "dynamic minimum" on these params to cover the overhead of fixed allocations, but I'm not sure how ugly that would get ?
Yes, the dynamic minimum sounds like a suitable concept, and the result looks pretty to me. Quick draft based on the example on the mailing list:
-
vsl_bufferstays as-is -
http_resp_sizestays as-is -
workspace_backendbecomes a dynamic minimum available to VCL and its default gets reduced by the currently deduced overhead for the other values.
The actual workspace size then becomes something like (please do not pick on details, those will are to be worked out and I do not want to spend the time before we agree in general):
PRNDUP(3 * HTTP_estimate(http_max_hdr) + vsl_buffer + http_resp_size)
discussed with @Dridi and @bsdphk :
- Unfortunately, the dynamic minimum is not a practical concept because it would lead to too low a setting when a dependent value is changed
- We want to change the semantics of the
workspace_*with a release to mean "usable space" - To help tune the parameters properly, we want to introduce a workspace high watermark which gets logged to vsl
-
WS_Release*will gain a parameter obliging the caller to report their high watermark - This change will happen with a major release
FTR previous PR #3285 needs to be redone
other bugwash notes on what to do once we have a watermark:
- Debug option for VSL reporting per task
- VSC reporting with simple histogram/bucketing (probably log-scale-ish like 50% 90% 95%)
#3524 is another incarnation of this. from bugwash
- need to look at dynamic parameters again, (it was only after bugwash that I noted the above Unfortunately, the dynamic minimum is not a practical concept...)
- we should snapshot all task-related values (like workspaces) in the worker
notes from bugwash:
(15:39:32) slink: I am back to thinking that just having headroom_xxx and auto-calculating the workspace_xxx would be the simplest and cleanest option (15:39:59) slink: workspace_xxx would then become r/o (15:41:42) slink: the reporting through the r/o parameters would also make any page size rounding obvious (15:41:59) phk: brew a mockup and lets look at it.