varnish-cache icon indicating copy to clipboard operation
varnish-cache copied to clipboard

not respecting implicit parameter dependencies triggers panics

Open nigoroll opened this issue 6 years ago • 6 comments

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.

nigoroll avatar Oct 21 '19 08:10 nigoroll

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 ?

bsdphk avatar Oct 21 '19 11:10 bsdphk

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_buffer stays as-is
  • http_resp_size stays as-is
  • workspace_backend becomes 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)

nigoroll avatar Oct 21 '19 12:10 nigoroll

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

nigoroll avatar Feb 07 '20 14:02 nigoroll

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%)

nigoroll avatar Oct 26 '20 14:10 nigoroll

#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

nigoroll avatar Feb 22 '21 14:02 nigoroll

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.

nigoroll avatar Aug 16 '21 13:08 nigoroll