moonchen opened a new issue, #13511:
URL: https://github.com/apache/trafficserver/issues/13511
In `plugins/experimental/rate_limit/limiter.h`, `RateLimiter::parseYaml()`:
```cpp
// ToDo: One or both of these should be required
const YAML::Node &queue = node["queue"];
// If enabled, we default to UINT32_MAX, but the object default is still 0
(no queue)
if (queue) {
_max_queue = queue["size"] ? queue["size"].as<uint32_t>() : UINT32_MAX;
```
A `queue:` block without a `size:` sets the cap to `UINT32_MAX`. `full()` is
`_size >= max_queue()`, so it can never be true and the admission path never
rejects; every over-limit connection is queued. The docs state the same at
`doc/admin-guide/plugins/rate_limit.en.rst:215`: "The size is default to
`UINT_MAX`, which is essentially unlimited."
Two problems follow.
**Resource holding.** For the SNI limiter a queued entry is a suspended TLS
handshake, so it holds a socket, a `NetVConnection`, an `SSL`, and the
handshake
buffers. A parked connection has its read VIO disabled, so the core cannot
notice the peer went away; nothing reaps it but
`proxy.config.ssl.handshake_timeout_in`, 30s by default. The effective
ceiling
is `proxy.config.net.connections_throttle`, 30000 by default, each held for
up
to 30s. For a plugin whose purpose is to bound resource consumption under
load,
that is inverted.
**Cost of removal.** `RateLimiter::remove()` is linear in the queue depth,
and
because `_queue` is a `std::deque` an erase from the middle is O(n) in
element
moves however the element is located.
Suggested fix: require the user to specify a queue size when a queue: block
is specified. Consider a different data structure for the queue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
