KAFKA-2837: fix transient failure of kafka.api.ProducerBounceTest > testBrokerFailure

[jinxing64]

I can reproduced this transient failure, it seldom happen;
code is like below:
// rolling bounce brokers
for (i <- 0 until numServers) {
for (server <- servers) {
server.shutdown()
server.awaitShutdown()
server.startup()
Thread.sleep(2000)
}

  // Make sure the producer do not see any exception
  // in returned metadata due to broker failures
  assertTrue(scheduler.failed == false)

  // Make sure the leader still exists after bouncing brokers
  (0 until numPartitions).foreach(partition => TestUtils.waitUntilLeaderIsElectedOrChanged(zkUtils, topic1, partition))

Brokers keep rolling restart, and producer keep sending messages;
In every loop, it will wait for election of partition leader;
But if the election is slow, more messages will be buffered in RecordAccumulator's BufferPool;
The limit for buffer is set to be 30000;
TimeoutException("Failed to allocate memory within the configured max blocking time") will show up when out of memory;
Since for every restart of the broker, it will sleep for 2000 ms, so this transient failure seldom happen;
But if I reduce the sleeping period, the bigger chance failure happens;
for example if the broker with role of controller suffered a restart, it will take time to select controller first, then select leader, which will lead to more messges blocked in KafkaProducer:RecordAccumulator:BufferPool;
In this fix, I just enlarge the producer's buffer size to be 1MB;
@guozhangwang , Could you give some comments?

[guozhangwang]

Why do you need to remove this config?

[guozhangwang]

@ZoneMayor Thanks for looking into this. I think this reasoning makes sense.

In the current producer version we have already deprecated METADATA_FETCH_TIMEOUT_CONFIG and BLOCK_ON_BUFFER_FULL_CONFIG, so in order to eliminate this issue instead of just reducing its likelihood we could choose to not set any of these two, and instead set MAX_BLOCK_MS_CONFIG to Long.MAX_VALUE so that the producer will be blocked forever if there is not enough data.

[jeanlyn]

👍 The reasoning also makes sense to me.

[guozhangwang]

Seems this variable is not used any more, could we remove it from createNewProducer as well?

[guozhangwang]

LGTM. Merged to trunk.

[ijuma]

@guozhangwang I don't understand the reasoning for setting MAX_BLOCK_MS_CONFIG to Long.MaxValue by default. Doesn't this mean that the test suite could hang instead of failing after a time out in case of bugs? Wouldn't it be better to set a time out that is high enough for our tests, but much lower than Long.MaxValue? Maybe 1 minute or something along those lines?

[guozhangwang]

@ijuma Yeah you are right. Setting it to MaxValue is not a good solution actually. I will submit a follow-up patch shortly.