----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29714/#review76500 -----------------------------------------------------------
core/src/main/scala/kafka/server/KafkaConfig.scala <https://reviews.apache.org/r/29714/#comment124033> Need to delete - Jeff Holoman On March 15, 2015, 5:13 a.m., Jeff Holoman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29714/ > ----------------------------------------------------------- > > (Updated March 15, 2015, 5:13 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1810 > https://issues.apache.org/jira/browse/KAFKA-1810 > > > Repository: kafka > > > Description > ------- > > KAFKA-1810 ConfigDef Refactor > > > Diffs > ----- > > core/src/main/scala/kafka/network/IPFilter.scala PRE-CREATION > core/src/main/scala/kafka/network/SocketServer.scala > 76ce41aed6e04ac5ba88395c4d5008aca17f9a73 > core/src/main/scala/kafka/server/KafkaConfig.scala > 46d21c73f1feb3410751899380b35da0c37c975c > core/src/main/scala/kafka/server/KafkaServer.scala > dddef938fabae157ed8644536eb1a2f329fb42b7 > core/src/test/scala/unit/kafka/network/IpFilterTest.scala PRE-CREATION > core/src/test/scala/unit/kafka/network/SocketServerTest.scala > 0af23abf146d99e3d6cf31e5d6b95a9e63318ddb > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > 191251d1340b5e5b2d649b37af3c6c1896d07e6e > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > 7f47e6f9a74314ed9e9f19d0c97931f3f2e49259 > > Diff: https://reviews.apache.org/r/29714/diff/ > > > Testing > ------- > > This code centers around a new class, CIDRRange in IPFilter.scala. The > IPFilter class is created and holds two fields, the ruleType > (allow|deny|none) and a list of CIDRRange objects. This is used in the Socket > Server acceptor thread. The check does an exists on the value in the list if > the rule type is allow or deny. On object creation, we pre-calculate the > lower and upper range values and store those as a BigInt. The overhead of the > check should be fairly minimal as it involves converting the incoming IP > Address to a BigInt and then just doing a compare to the low/high values. In > writing this review up I realized that I can optimize this further to convert > to bigint first and move that conversion out of the range check, which I can > address. > > Testing covers the CIDRRange and IPFilter classes and validation of IPV6, > IPV4, and configurations. Additionally the functionality is tested in > SocketServerTest. Other changes are just to assist in configuration. > > I modified the SocketServerTest to use a method for grabbing the Socket > server to make the code a bit more concise. > > One key point is that, if there is an error in configuration, we halt the > startup of the broker. The thinking there is that if you screw up > security-related configs, you want to know about it right away rather than > silently accept connections. (thanks Joe Stein for the input). > > There are two new exceptions realted to this functionality, one to handle > configuration errors, and one to handle blocking the request. Currently the > level is set to INFO. Does it make sense to move this to WARN ? > > > Thanks, > > Jeff Holoman > >
