-----------------------------------------------------------
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 (updated)
-------

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 (updated)
-----

  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

Reply via email to