> On March 12, 2015, 1:04 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/utils/TestUtils.scala, line 67 > > <https://reviews.apache.org/r/31806/diff/2/?file=889521#file889521line67> > > > > The name is a bit misleading since the port value is actually fixed. > > Maybe we can renamed it to "DefaultZKPort".
The constant has a fixed value, but it generates a random port. Passing 0 tells the OS "give me a random port". I actually named the constant specifically so it would be clear that 0 wasn't the port. I've added a comment which hopefully clarifies this. > On March 12, 2015, 1:04 p.m., Guozhang Wang wrote: > > core/src/test/scala/unit/kafka/utils/TestUtils.scala, lines 157-160 > > <https://reviews.apache.org/r/31806/diff/2/?file=889521#file889521line157> > > > > Is it OK for the zkConnect's port to be the same to the server port > > (i.e. 1)? It's definitely not ok, but I'm not sure what you're trying to get at here? We could check that they don't conflict, but most of the test harness code handles that, adding a check here doesn't handle any conflicts between broker port assignments if they pass in a value for the port argument, and it should be very obvious that they conflict now since any port conflicts are can actually be attributed to a real conflict after this patch is applied. - Ewen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31806/#review76190 ----------------------------------------------------------- On March 9, 2015, 6:41 p.m., Ewen Cheslack-Postava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31806/ > ----------------------------------------------------------- > > (Updated March 9, 2015, 6:41 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1501 > https://issues.apache.org/jira/browse/KAFKA-1501 > > > Repository: kafka > > > Description > ------- > > This removes the TestUtils.choosePorts and TestZKUtils utilities because the > ports they claim to allocate can't actually be guaranteed to work. Instead, we > allow the port to be 0 to make the kernel give us a random port. This is only > useful in tests, but ensures we'll always be able to bind a socket as long as > some ports are still available. > > The impact on the main code is fairly minimal, but we also have to be careful > about using the advertisedPort setting since it defaults to the port setting, > which may no longer represent the actual port. To support this case and so > tests > are able to discover the port the server was bound to, we now provide a > boundPort method on the server. > > Most of the changes to the tests are straightforward adaptations. A few > settings > that were previously just val fields must now be methods because their value > depends on the port value, which won't be known until setUp() starts the > servers. The biggest impact of this is that we cannot generate broker configs > during the test class initialization. Instead, KafkaServerTestHarness now > provides a hook that classes implement to create configs and a method that > gets > them that is compatible with the old field version in order to keep code > changes > to a minimum. > > Fix testBrokerFailure test to better handle the changing broker addresses > caused by bouncing the servers when using randomly allocated ports. > > > Temporarily disable testBrokerFailure test. > > > Minor cleanup based on review. > > > Diffs > ----- > > clients/src/test/java/org/apache/kafka/common/network/SelectorTest.java > 0d030bc9becfa7db5b27ecf1df03eb71074f92d3 > clients/src/test/java/org/apache/kafka/test/TestUtils.java > 20dba7b9199273ca8952c4fea71efadc2f09f044 > core/src/main/scala/kafka/network/SocketServer.scala > 76ce41aed6e04ac5ba88395c4d5008aca17f9a73 > core/src/main/scala/kafka/server/KafkaServer.scala > 378a74d9e8e408e1e5d283badf3eded6333fadff > core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala > 82fe4c9a138617f7af99a54cca7176d6c80747d0 > core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala > cae72f4f87f10b843c29dc731c6e0028b1d50734 > core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala > 8246e1281097e33eb8fadb291dc5feefdb631515 > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > 3df450784592b894008e7507b2737f9bb07f7bd2 > core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala > 8bc178505e00932a316c46ed1d904bd57b5b3f75 > core/src/test/scala/unit/kafka/admin/AdminTest.scala > ee0b21e6a94ad79c11dd08f6e5adf98c333e2ec9 > core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala > 1baff0ea9826495c85c28763deeed78d052728fa > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala > 6258983451b0ff5dfdc5e79be47c90a17525e284 > core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala > 995397ba2e2dfc6fadd9d5c5efd90f2c4ac0d59c > > core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala > 19640cc55b5baa0a26a808d708b7f4caf491c9f0 > core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala > ffa6c306a44311296fb182a61529e5168f0a84c4 > core/src/test/scala/unit/kafka/integration/FetcherTest.scala > 3093e459935ecf8e5b34fca34a422674562a7034 > core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala > dc0512b526e914df7e7581b27df18f498da428e2 > core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala > 30deaf47b64592f2e1cc84a4156671fac11b67ef > > core/src/test/scala/unit/kafka/integration/ProducerConsumerTestHarness.scala > 108c2e7f47ede038855e7fa3c3df582d86e8c5c3 > core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala > 4d27e41c727e73544b2b214a0a0b60f6acdbfd17 > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala > a671af4a87d5c2fb42ff48c553bca7cae6538231 > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala > 8342cae564ebc39fe74a512343a4523072ca205a > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala > 3d0fc9deda2d3a39f2618a5be3edd98cd935ffbb > core/src/test/scala/unit/kafka/log/LogTest.scala > 8cd5f2fa4a1a536c3983c5b6eac3d80de49d5a94 > core/src/test/scala/unit/kafka/log4j/KafkaLog4jAppenderTest.scala > 36db9172ea2d4d7e242e023ba914596c1f64f5f4 > core/src/test/scala/unit/kafka/metrics/MetricsTest.scala > 0f58ad8e698e3c0ec76c510bd5f76912a992209c > core/src/test/scala/unit/kafka/network/SocketServerTest.scala > 0af23abf146d99e3d6cf31e5d6b95a9e63318ddb > core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala > be90c5bc7f1f5ba8a237d1c7176f27029727c918 > core/src/test/scala/unit/kafka/producer/ProducerTest.scala > d2f3851cb68ff595bbb859d27b3552e751d060bb > core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala > b5208a5f1186bc089cd89527c1eb7f95b2e76c75 > core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala > 296e2b563041318d360db4037411879613a63968 > core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala > 93182aeb342729d420d2e7d59a1035994164b7db > core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala > 0bdbc2f31fcac2abdeff4e41e448a7b9ab13b95b > core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala > 92152358c95fa9178d71bd1c079af0a0bd8f1da8 > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > 7f47e6f9a74314ed9e9f19d0c97931f3f2e49259 > core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala > f2528059dd86bb051d1ce9d72039cde3b78b8817 > core/src/test/scala/unit/kafka/server/LogOffsetTest.scala > 8c9f9e748e888da6035755c9f4241ac3aa0500c9 > core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala > 92d6b2c672f74cdd526f2e98da8f7fb3696a88e3 > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > ea9b315099254eca82b8d5534bc02535b5996ee9 > core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala > 1e64faf9a7e6283822490d6d737ad8871248e27d > core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala > 2849a5e7607910fe20e58685ab7ad49a7e7db8e8 > core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala > 96a8a5a8cb1f40f743490f85bf18d441ce86a765 > core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala > b46daa436231d5aa5c1e2992fd5c2d9a73a30c80 > core/src/test/scala/unit/kafka/server/ServerStartupTest.scala > 60021ef8d8d7fa172d6b160db48ee5eb9574e1a9 > core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala > efb457334bd87e4c5b0dd2c66ae1995993cd0bc1 > core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala > 2edc814e7a99c544f5ce7bfdbb4c3fd335ef3a51 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 6ce18076f6b5deb05b51c25be5bed9957e6b4339 > core/src/test/scala/unit/kafka/zk/EmbeddedZookeeper.scala > 31515615089385c722325bfe019a47ae3a9a4052 > core/src/test/scala/unit/kafka/zk/ZKPathTest.scala > 9897b2fa8f8261fe8ab6b62b45b9052adb07043f > core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala > 67d9c4bab270c852dc05d47aaa4dd96b0f6039d4 > > Diff: https://reviews.apache.org/r/31806/diff/ > > > Testing > ------- > > > Thanks, > > Ewen Cheslack-Postava > >
