Change Details

The current implementation of haproxy per-client-IP concurrency limitation is messy, with tightly-coupled configuration stanzas spanning multiple different parts of its [[ https://gerrit.wikimedia.org/g/operations/puppet/+/310baf3d957c6d9acd32b3a8f0599b5e6f3277b1/hieradata/common/profile/cache/haproxy.yaml#67 | hiera ]]: * A sticktable (what haproxy calls this kind of hashtable) is defined in `profile::cache::haproxy::sticktables`; we name it `httpreqrate`. * `profile::cache::haproxy::pre_acl_actions` contains an entry that tracks in-flight concurrency of each client IP on an incoming HTTP request ** This references the sticktable by name ** This also references a specific "stickycounter" used for highly-efficient tracking: in this case `sc0`. * Multiple entries in `profile::cache::haproxy::acls` record our desired behavior ** Thresholds/business logic (e.g. "2000") live here ** ACLs also reference `sc0` by name ** and reference the sticktable by name `httpreqrate` * One entry in `profile::cache::haproxy::post_acl_actions` glues everything together and performs enforcement; it references the ACLs by name (and thus causes them to be evaluated) * One entry in `profile::cache::haproxy::vars` -- a dummy variable not actually referenced elsewhere -- is used for debug logging output when an IP first starts being temporarily blocked. ** This references some of the ACLs by name. It might be better to abstract this out so that you only need define one "thing", and the above config artifacts are then generated for you. It seems very likely we'll want to create further rules that follow the pattern "when $X threshold on $Y metric is exceeded, log it, and temporarily start doing $SOMETHING_ELSE with related traffic". This would have a few advantages: * Much easier to create new ones without risking referencing something by the wrong name * Could easily support a "dry run"/"only log" mode * Easy to support and switch between multiple $SOMETHING_ELSEes Downsides: * More complicated Puppet code * Potential complications with merging the 'raw' hiera with generated-from-other-hiera stuff * Less flexibility?

The current implementation of haproxy per-client-IP concurrency limitation is messy, with tightly-coupled configuration stanzas spanning multiple different parts of its [[ https://gerrit.wikimedia.org/g/operations/puppet/+/310baf3d957c6d9acd32b3a8f0599b5e6f3277b1/hieradata/common/profile/cache/haproxy.yaml#67 | hiera ]]: * A sticktable (what haproxy calls this kind of hashtable) is defined in `profile::cache::haproxy::sticktables`; we name it `httpreqrate`. * `profile::cache::haproxy::pre_acl_actions` contains an entry that tracks in-flight concurrency of each client IP on an incoming HTTP request ** This references the sticktable by name ** This also references a specific "stickycounter" used for highly-efficient tracking: in this case `sc0`. * Multiple entries in `profile::cache::haproxy::acls` record our desired behavior ** Thresholds/business logic (e.g. "2000") live here ** ACLs also reference `sc0` by name ** and reference the sticktable by name `httpreqrate` * One entry in `profile::cache::haproxy::post_acl_actions` glues everything together and performs enforcement; it references the ACLs by name (and thus causes them to be evaluated) * One entry in `profile::cache::haproxy::vars` -- a dummy variable not actually referenced elsewhere -- is used for debug logging output when an IP first starts being temporarily blocked. ** This references some of the ACLs by name. It might be better to abstract this out so that you only need define one "thing", and the above config artifacts are then generated for you. It seems very likely we'll want to create further rules that follow the pattern "when $X threshold on $Y metric is exceeded, log it once, and temporarily start doing $SOMETHING_ELSE with related traffic". This would have a few advantages: * Much easier to create new ones without risking referencing something by the wrong name * Could easily support a "dry run"/"only log" mode * Easy to support and switch between multiple $SOMETHING_ELSEes Downsides: * More complicated Puppet code * Potential complications with merging the 'raw' hiera with generated-from-other-hiera stuff * Less flexibility?

The current implementation of haproxy per-client-IP concurrency limitation is messy, with tightly-coupled configuration stanzas spanning multiple different parts of its [[ https://gerrit.wikimedia.org/g/operations/puppet/+/310baf3d957c6d9acd32b3a8f0599b5e6f3277b1/hieradata/common/profile/cache/haproxy.yaml#67 | hiera ]]: * A sticktable (what haproxy calls this kind of hashtable) is defined in `profile::cache::haproxy::sticktables`; we name it `httpreqrate`. * `profile::cache::haproxy::pre_acl_actions` contains an entry that tracks in-flight concurrency of each client IP on an incoming HTTP request ** This references the sticktable by name ** This also references a specific "stickycounter" used for highly-efficient tracking: in this case `sc0`. * Multiple entries in `profile::cache::haproxy::acls` record our desired behavior ** Thresholds/business logic (e.g. "2000") live here ** ACLs also reference `sc0` by name ** and reference the sticktable by name `httpreqrate` * One entry in `profile::cache::haproxy::post_acl_actions` glues everything together and performs enforcement; it references the ACLs by name (and thus causes them to be evaluated) * One entry in `profile::cache::haproxy::vars` -- a dummy variable not actually referenced elsewhere -- is used for debug logging output when an IP first starts being temporarily blocked. ** This references some of the ACLs by name. It might be better to abstract this out so that you only need define one "thing", and the above config artifacts are then generated for you. It seems very likely we'll want to create further rules that follow the pattern "when $X threshold on $Y metric is exceeded, log it once, and temporarily start doing $SOMETHING_ELSE with related traffic". This would have a few advantages: * Much easier to create new ones without risking referencing something by the wrong name * Could easily support a "dry run"/"only log" mode * Easy to support and switch between multiple $SOMETHING_ELSEes Downsides: * More complicated Puppet code * Potential complications with merging the 'raw' hiera with generated-from-other-hiera stuff * Less flexibility?