----------------------------------------------------------- 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 > >
