KAFKA-17999: Fix flaky DynamicConnectionQuotaTest testDynamicConnectionQuota#21354
KAFKA-17999: Fix flaky DynamicConnectionQuotaTest testDynamicConnectionQuota#21354chickenchickenlove wants to merge 2 commits intoapache:trunkfrom
Conversation
github-actions
bot
added
triage
|
@mumrah Hi! |
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
|
@mumrah |
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
|
@chia7712 Hi! sorry to mention suddenly. |
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
|
Thanks for the PR. Can you rebase on trunk to trigger the CI? |
chickenchickenlove
force-pushed
the
KAFKA-17999
branch
from
9b8aeee to
087cea8
Compare
|
@mimaison |
|
A label of 'needs-attention' was automatically added to this PR in order to raise the |
mimaison
left a comment
mimaison
left a comment
There was a problem hiding this comment.
Can you also remove the Flaky annotation on the test?
| val maxConnectionsPerIPOverride = 7 | ||
| props.put(SocketServerConfigs.MAX_CONNECTIONS_PER_IP_OVERRIDES_CONFIG, s"localhost:$maxConnectionsPerIPOverride") | ||
| reconfigureServers(props, perBrokerConfig = false, (SocketServerConfigs.MAX_CONNECTIONS_PER_IP_OVERRIDES_CONFIG, s"localhost:$maxConnectionsPerIPOverride")) | ||
| waitForMaxConnectionsOverrideApplied("127.0.0.1", maxConnectionsPerIPOverride) |
There was a problem hiding this comment.
Would it make more sense to use localhost like above?
There was a problem hiding this comment.
You are right! I will address it!
|
@mimaison thanks for taking the time to look at this! |
2 participants
Summary
DynamicConnectionQuotaTest#testDynamicConnectionQuotaby waiting untilthe broker’s
max.connections.per.ip.overridesis actually applied inConnectionQuotasbefore running the second verification.Background
testDynamicConnectionQuotadynamically updatesmax.connections.per.ip.overridesfrom 5 to 7 and immediately startsverifyMaxConnections(7, ...).In practice, the config update is propagated asynchronously: the broker
SocketServerreconfigure is triggered, but there is a short windowwhere the data-plane acceptor still evaluates new connections using the
old override state(i.e, 5). This can cause one of the “prefill” sockets
in the second verification to be rejected under
max=5, breaking thetest’s assumptions and leading to intermittent failures.
Changes
ConnectionQuotasto check whether anoverride for a given IP is present.
DynamicConnectionQuotaTest, after alteringmax.connections.per.ip.overrides, wait until the broker’sConnectionQuotasobserves 127.0.0.1 -> 7 before executing the secondverifyMaxConnections(...).Reproduction / Verification
testDynamicConnectionQuotawhen the override was not yet applied during the second verification.
max=7 and the test becomes deterministic.
Test Fail Ratio in my local.
Flaky / Success diagram
Reviewers: Mickael Maison mimaison@users.noreply.github.com