junrao commented on code in PR #14836:
URL: https://github.com/apache/kafka/pull/14836#discussion_r1409739363
##########
core/src/test/scala/unit/kafka/server/BrokerLifecycleManagerTest.scala:
##########
@@ -201,7 +201,7 @@ class BrokerLifecycleManagerTest {
while (!future.isDone || context.mockClient.hasInFlightRequests) {
context.poll()
manager.eventQueue.wakeup()
- context.time.sleep(100)
+ context.time.sleep(5)
Review Comment:
@soarez : Thanks for the explanation. You are right that `KafkaEventQueue`
does de-duping and only allows one outstanding `CommunicationEvent` in the
queue. But it seems that duplicated `HeartbeatRequest`s could still be
generated, which is causing the original transient failure.
`CommunicationEvent` calls `sendBrokerHeartbeat` that calls the following.
`_channelManager.sendRequest(new BrokerHeartbeatRequest.Builder(data),
handler)
`
The problem is that we have another queue in
`NodeToControllerChannelManagerImpl` that doesn't do the de-duping. Once a
`CommunicationEvent` is dequeued from `KafkaEventQueue`, a `HeartbeatRequest`
will be queued in `NodeToControllerChannelManagerImpl`. At this point, another
`CommunicationEvent` could be enqueued in `KafkaEventQueue`. When it's
processed, another `HeartbeatRequest` will be queued in
`NodeToControllerChannelManagerImpl`.
This probably won't introduce long lasting duplicated `HeartbeatRequest` in
practice since `CommunicationEvent` is typically queued in `KafkaEventQueue`
for heartbeat interval. By that time, other pending `HeartbeatRequest`s will be
processed and de-duped when enqueuing to `KafkaEventQueue`. But, maybe we could
file a jira to track it.
For the test, could we add a comment to explain why we need to wait for 10
`HeartbeatRequest`s?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
