Implementation of RateLimitNetworkTrafficListener #406
Conversation
| "network.traffic.rate.limit.backend"; | ||
| protected static final String NETWORK_TRAFFIC_RATE_LIMIT_BACKEND_DOC = | ||
| "The rate-limiting backend to use. The options are 'guava', and 'resilience4j'." | ||
| + "Default is 'guava'."; |
There was a problem hiding this comment.
For reference, the kafka-rest default may be guava (and good we match it), but we actually run with the config set to resliance4j, so the launch darkly setting should match that.
There was a problem hiding this comment.
Yeah, I'll make the config in LD set to resilience4j
There was a problem hiding this comment.
What is the reason to keeping both guava/resilience4j as 2 options? I never understood why kafka-rest gives an option.
IMO, ideally whatever the reason, would have converged to 1 option by now, no reason to have LD/config.
There was a problem hiding this comment.
it is some historical reason I think when it first developed
core/src/main/java/io/confluent/rest/RateLimitNetworkTrafficListener.java
Show resolved
Hide resolved
core/src/test/java/io/confluent/rest/RateLimitNetworkTrafficListenerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
@trnguyencflt This mostly looks good, thanks for doing the change!
Most of the comments are nits, but is it possible to improve the test? see my comment above.
core/src/main/java/io/confluent/rest/RateLimitNetworkTrafficListener.java
Show resolved
Hide resolved
…d change the test to show drastic effect of rate limiting
|
@msn-tldr I have addressed the comments, could you please have a look again? |
core/src/test/java/io/confluent/rest/RateLimitNetworkTrafficListenerTest.java
Outdated
Show resolved
Hide resolved
core/src/test/java/io/confluent/rest/RateLimitNetworkTrafficListenerTest.java
Outdated
Show resolved
Hide resolved
This PR implements a method to slowdown the reading rate from HTTP/TCP socket in Jetty. The approach is based on the fact that we can use NetworkTrafficListener to count for the reading from TCP sockets on all connections. This introduces 3 new configurations (there will be changed in cc-spec-kafka):
network.traffic.rate.limit.enable: whether to enable this rate limitnetwork.traffic.rate.limit.backend: backend of rate limiting, either guava or resilience4jnetwork.traffic.rate.limit.bytes.per.sec: the rate to limit, default to 20MiBReference JIRA https://confluentinc.atlassian.net/browse/KREST-11222