splett2 commented on a change in pull request #9386:
URL: https://github.com/apache/kafka/pull/9386#discussion_r523682718
##########
File path: core/src/test/scala/unit/kafka/network/ConnectionQuotasTest.scala
##########
@@ -633,18 +837,27 @@ class ConnectionQuotasTest {
listenerName: ListenerName,
address: InetAddress,
numConnections: Long,
- timeIntervalMs: Long) : Unit = {
+ timeIntervalMs: Long,
+ expectIpThrottle: Boolean): Boolean = {
var nextSendTime = System.currentTimeMillis + timeIntervalMs
+ var ipThrottled = false
for (_ <- 0L until numConnections) {
// this method may block if broker-wide or listener limit on the number
of connections is reached
- connectionQuotas.inc(listenerName, address,
blockedPercentMeters(listenerName.value))
-
+ try {
+ connectionQuotas.inc(listenerName, address,
blockedPercentMeters(listenerName.value))
+ } catch {
+ case e: ConnectionThrottledException =>
+ if (!expectIpThrottle)
+ throw e
+ ipThrottled = true
Review comment:
I took a closer look at some of the IP throttling tests, and I think
they are a little too coarse-grained with respect to testing the quota rate
being enforced. I updated the IP throttling rate above/below tests that don't
expect to block to use `acceptConnectionsAndVerifyRate` with `MockTime` to
verify that the expected quota rate holds while getting throttled.
It's a little messy, because a lot of the other tests for broker/listener
quotas have to use `Time.SYSTEM` due to monitor waiting. I think it should also
be possible to rewrite the broker/listener connection rate quota tests to use
mock time by extending the `Time` interface, but I would prefer to do that in a
follow-up PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
