kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gwen Shapira" <gshap...@cloudera.com>
Subject Re: Review Request 29029: Patch for KAFKA-1512
Date Mon, 15 Dec 2014 19:58:25 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29029/#review65114
-----------------------------------------------------------


Jeff,

There's some strange things going on there, I think we need a bit more testing and maybe more
implementation :)

1. Please make sure the test fails without the override fix. When I tried, it passed on trunk...
this means we are testing the wrong thing.

2. Funny fact: connect() does not actually trigger the Quota mechanism, it is only triggered
when you send a request. You can see that by putting a breakpoint in ConnectionQuotas.inc
and see where it is called. Since you are only sending data after creating the "last" connection,
even without the override you'll be able to create the first 5 connections and only get the
error after the 6 one and the "send request"... this is probably why the test works with and
without the override.

I'm not sure, but this may be a bug in the original maxIP implementation - since I can actually
create gazillion connections as long as I don't send anything. I'm not sure if Kafka could
run out of resources in this case. Perhaps check with Jay in the JIRA? He probably thought
about this.

3. Not sure, but perhaps we need to call fail() explicitly to make sure the test fails if
we successfully openned the last connection and sent data?

4. Another funny fact: ((0 until overrideNum).map(i => connect())) creates 6 connections,
not 5

5. We need to make sure the "overrides" map is propagated all the way to the ConnectionQuotas
code. I don't think it does that at the moment, even after you fixed the SocketServer() call.

Thanks again for your work here, and sorry it got slightly more complex than expected.

- Gwen Shapira


On Dec. 15, 2014, 2:30 a.m., Jeff Holoman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29029/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2014, 2:30 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1512
>     https://issues.apache.org/jira/browse/KAFKA-1512
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1512 wire in Override configuration
> 
> 
> KAFKA-1512 wire in overrides
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/network/SocketServer.scala e451592fe358158548117f47a80e807007dd8b98

>   core/src/main/scala/kafka/server/KafkaServer.scala 1bf7d10cef23a77e716666eb16bf6d0e68bc4ebe

>   core/src/test/scala/unit/kafka/network/SocketServerTest.scala 5f4d85254c384dcc27a5a84f0836ea225d3a901a

> 
> Diff: https://reviews.apache.org/r/29029/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jeff Holoman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message