Status quo
Right now, we generate haproxy config language like this to implement silent-drop:
# Pseudo-backends used only for statistics tracking. backend httpreqrate stick-table type ipv6 size 1m expire 300s store http_req_rate(10s),gpc0_rate(300s) backend httpreqrate_http stick-table type ipv6 size 1m expire 300s store http_req_rate(10s),gpc0_rate(300s) listen tls http-request track-sc0 src table httpreqrate # the instantaneous value acl too_many_concurrent_queries sc0_trackers(httpreqrate) ge 500 # a counter incremented the first time instantaneous above a threshold within a given "rate" window (per above, 300s) acl too_much_recent_concurrency sc0_gpc0_rate(httpreqrate) gt 0 # yes, this next one is mutating to evaluate, and yes, that is haproxy-idiomatic acl mark_as_too_much_concurrency sc0_inc_gpc0(httpreqrate) gt 0 # "Edge-triggered" logging: log the first time the constraint is violated, then stop http-request set-var(req.dummy_silent_drop) src,debug(silent-drop_for_300s,stderr) if too_many_concurrent_queries !too_much_recent_concurrency # Enforcement http-request silent-drop if too_much_recent_concurrency || too_many_concurrent_queries mark_as_too_much_concurrency
This device is a modification of one often shown in official haproxy configuration examples. It adds short-term memory by using gpc0_rate -- the rate-of-change-over-fixed-time-window of a general-purpose counter. This allows us to log a message only the first time that a given src exceeds its allowed concurrency, which greatly limits the number of log messages produced.
Make it better
We can generalize this device, and allow for multiple different threshold values with the same underlying stick-table and stickycounter (sc0).
We can also allow for a "dry run" mode, so we can test new thresholds or different triggering rules in production without actually affecting traffic.
These are two features we've wanted for a long time; see T329331 and T353910
Implementation proposal
- change the stick-table ... store ...,gpc0_rate(300s) definitions to instead gpc_rate(N,300s). This allows for an array of N elements. To start with we could choose something like 10?
- Expanding past 10 in the future is not hard, if we need to
- This change is backwards-compatible with the current ACLs as written, so it can be rolled out in Puppet any time (well, maybe we need to first double-check that changing a stick-table store definition is allowed at runtime)
- Keep stick-table definitions and any necessary trackers like track-sc0 defined statically in puppet
- Add to requestctl a new flavor of haproxy_action that can generate the correct acl definitions and the necessary http-request verbs
- Dry run can be implemented by moving the stanzas now after the || of the final line into its own set-var
- This would allow for us to still log to syslog that the concurrency limit would have kicked in, without being spammy.
- Use one array index in the gpc for hysteresis for a given action.
- Requestctl can persist the meaning of the array numbers, likely as extra state in etcd.
- Upon activation, an action receives an array index known to be free, persist this in etcd. Error out if we can't find one available to allocate.
- Upon de-activation of an action, we note the timestamp of the deactivation, and persist in etcd. Then the index is free to re-use at any time 5 minutes after that timestamp.
Open questions
- Do we want some way to express haproxy sample fetches like sc0_trackers as requestctl patterns? Or would it be better to have statically-defined or defined-in-config patterns that provide an API surface for concurrency & any other things we are tracking in stick-tables?
- Or should we do a very minimal option for now by essentially hardcoding the concurrency measurement "pattern" within the new action?