> 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". > > Ewen Cheslack-Postava wrote: > 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.
I see; was misreading the code before, also the comment below came from this misreading. - Guozhang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31806/#review76190 ----------------------------------------------------------- On March 25, 2015, 7:45 a.m., Ewen Cheslack-Postava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31806/ > ----------------------------------------------------------- > > (Updated March 25, 2015, 7:45 a.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. > > > Move testBrokerFailure to its own test that uses the old choosePorts approach > to allocating ports since its correctness depends on the broker maintaining > the same address across a bounce. > > > Address review comments. > > > Refactor bounce tests from ConsumerTest to use fixed ports. > > This includes a class to share this functionality, much as it used to be > shared > from TestUtils, since it is now needed by both producer and consumer bounce > tests. However, it comes with a big warning not to use it unless you have a > very > good reason since it can result in port conflicts. > > > 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 > dddef938fabae157ed8644536eb1a2f329fb42b7 > core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala > PRE-CREATION > core/src/test/scala/integration/kafka/api/ConsumerTest.scala > c82bdaad131c08b241541afb79e79355d3e0ec36 > core/src/test/scala/integration/kafka/api/FixedPortTestUtils.scala > PRE-CREATION > core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala > 5b7e36695aedebeb14a0d5d5d16d3097852d8bdc > core/src/test/scala/integration/kafka/api/ProducerBounceTest.scala > PRE-CREATION > core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala > cae72f4f87f10b843c29dc731c6e0028b1d50734 > core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala > 7eb6d05b66b16f8b0ab64eb43dcc1a25168fa5be > 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 > 155fd046c9036b5b8b107bd21a8c6492c14644ea > 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 > 062790f15d4ab6e0ee7a98bef12aaa2b22da3bfa > 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 > e4d0435eb4213597c2fb9c3f2093c227de53a417 > 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 > 1682a77362123449de6bb1d54a55887409990a24 > 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 > >
