> On July 6, 2014, 7:34 p.m., Jun Rao wrote: > >
I think what I have may be right. My understanding is that - InetAddress is an IP address - SocketAddress, InetSocketAddress are host/port pairs (basically) getInetAddress gets the remote ip address whereas getRemoteAddress gets the InetSocketAddress. This is sort of crazy naming, but that is my understanding. My thought was that it was better to work with InetAddress rather than host because host can be ambiguously defined (e.g. app301, app301.prod, app301.prod.linkedin.com, etc). > On July 6, 2014, 7:34 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, lines 477-478 > > <https://reviews.apache.org/r/23208/diff/2/?file=623799#file623799line477> > > > > Would it be useful to add some metrics such as avg/max connections per > > host? I think avg is not useful, and will be misleading in the case of a leak (e.g. one host with many, many connections). I think max would be useful, however it is a bit of a pain. The three options I could think of: - add a gauge that iterates over all hosts in the map and computes the max. In the common case this will be fine, but it is O(N) in the number of hosts. So if you have a ton of connections and a low poll time for your metrics (some people use 1 sec) then you will spend something like 30% of one cpu core counting connections while holding the quota lock. In general I think we have tried to avoid operations that iterate over all hosts or connections to ensure the socket server scales to lots of connections without slowing down. - Add a SortedSet on (count, ip) that gets adjusted on inc and dec. This is correct and O(1)ish but is just complicated. - Implement a broken max that only goes up. This would be easily but annoying. > On July 6, 2014, 7:34 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, lines 255-256 > > <https://reviews.apache.org/r/23208/diff/2/?file=623799#file623799line255> > > > > We probably should get remoteSocketAddress? See above. > On July 6, 2014, 7:34 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/network/SocketServer.scala, lines 478-479 > > <https://reviews.apache.org/r/23208/diff/2/?file=623799#file623799line478> > > > > An InetAddress has a host and a port. It seems that we need to key on > > the hostName of an InetAddress. See above. > On July 6, 2014, 7:34 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/server/KafkaConfig.scala, lines 98-99 > > <https://reviews.apache.org/r/23208/diff/2/?file=623800#file623800line98> > > > > This may not work well with ipv6 since the ip will contain ":". This is > > also an issue for broker.list. So, do we want to address the issue now or > > fix it later together with broker.list? This is a really good point. I don't care too much about this option as it is going to be rarely used. What I think is more important is that we fix the option in the new producer/consumer before we do an official release. > On July 6, 2014, 7:34 p.m., Jun Rao wrote: > > core/src/test/scala/unit/kafka/network/SocketServerTest.scala, line 153 > > <https://reviews.apache.org/r/23208/diff/2/?file=623802#file623802line153> > > > > Should we just fail the test if we reach here? Technically I think there is a race condition and either the exception or the read with -1 is possible, so I think you need both. - Jay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23208/#review47362 ----------------------------------------------------------- On July 3, 2014, 10:18 p.m., Jay Kreps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23208/ > ----------------------------------------------------------- > > (Updated July 3, 2014, 10:18 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1512 > https://issues.apache.org/jira/browse/KAFKA-1512 > > > Repository: kafka > > > Description > ------- > > KAFKA-1512 Add per-ip connection limits. > > > Diffs > ----- > > core/src/main/scala/kafka/network/SocketServer.scala > 4976d9c3a66bc965f5870a0736e21c7b32650bab > core/src/main/scala/kafka/server/KafkaConfig.scala > ef75b67b67676ae5b8931902cbc8c0c2cc72c0d3 > core/src/main/scala/kafka/server/KafkaServer.scala > c22e51e0412843ec993721ad3230824c0aadd2ba > core/src/test/scala/unit/kafka/network/SocketServerTest.scala > 1c492de8fde6582ca2342842a551739575d1f46c > > Diff: https://reviews.apache.org/r/23208/diff/ > > > Testing > ------- > > > Thanks, > > Jay Kreps > >
