-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-5358: Consumer perf tool should count rebalance time. #3188
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
be43bf3
cbdf6c1
7600e6b
08ff452
8c80f13
edf1d08
722e16d
57bd0e4
16cc2ca
3798272
bbba9b0
3a1ce0f
c517815
9adda74
dfa9422
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,10 +54,12 @@ object ConsumerPerformance { | |
| val totalBytesRead = new AtomicLong(0) | ||
| val consumerTimeout = new AtomicBoolean(false) | ||
| var metrics: mutable.Map[MetricName, _ <: Metric] = null | ||
| val joinGroupTimeInMs = new AtomicLong(0) | ||
| val fetchTimeInMs = new AtomicLong(0) | ||
|
|
||
| if (!config.hideHeader) { | ||
| if (!config.showDetailedStats) | ||
| println("start.time, end.time, data.consumed.in.MB, MB.sec, data.consumed.in.nMsg, nMsg.sec") | ||
| println(s"start.time, end.time, data.consumed.in.MB, MB.sec, data.consumed.in.nMsg, nMsg.sec${if (!config.useOldConsumer) ", total.rebalance.time.ms, fetch.time.ms" else ""}") | ||
| else | ||
| println("time, data.consumed.in.MB, MB.sec, data.consumed.in.nMsg, nMsg.sec") | ||
| } | ||
|
|
@@ -67,7 +69,8 @@ object ConsumerPerformance { | |
| val consumer = new KafkaConsumer[Array[Byte], Array[Byte]](config.props) | ||
| consumer.subscribe(Collections.singletonList(config.topic)) | ||
| startMs = System.currentTimeMillis | ||
| consume(consumer, List(config.topic), config.numMessages, 1000, config, totalMessagesRead, totalBytesRead) | ||
| consume(consumer, List(config.topic), config.numMessages, 1000, | ||
| config, totalMessagesRead, totalBytesRead, joinGroupTimeInMs) | ||
| endMs = System.currentTimeMillis | ||
|
|
||
| if (config.printMetrics) { | ||
|
|
@@ -98,10 +101,17 @@ object ConsumerPerformance { | |
| consumerConnector.shutdown() | ||
| } | ||
| val elapsedSecs = (endMs - startMs) / 1000.0 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the main point of this patch is to compute the throughput statistics based only on the time spent fetching, so maybe we can move this below and just use |
||
| fetchTimeInMs.set((endMs - startMs) - joinGroupTimeInMs.get) | ||
| if (!config.showDetailedStats) { | ||
| val totalMBRead = (totalBytesRead.get * 1.0) / (1024 * 1024) | ||
| println("%s, %s, %.4f, %.4f, %d, %.4f".format(config.dateFormat.format(startMs), config.dateFormat.format(endMs), | ||
| totalMBRead, totalMBRead / elapsedSecs, totalMessagesRead.get, totalMessagesRead.get / elapsedSecs)) | ||
| println(s"%s, %s, %.4f, %.4f, %d, %.4f${if (!config.useOldConsumer) ", %s" else ""}".format( | ||
| config.dateFormat.format(startMs), | ||
| config.dateFormat.format(endMs), | ||
| totalMBRead, | ||
| totalMBRead / elapsedSecs, | ||
| totalMessagesRead.get, | ||
| totalMessagesRead.get / elapsedSecs, | ||
| if (!config.useOldConsumer) joinGroupTimeInMs.get.toString + ", " + fetchTimeInMs.get.toString else "")) | ||
| } | ||
|
|
||
| if (metrics != null) { | ||
|
|
@@ -110,25 +120,28 @@ object ConsumerPerformance { | |
|
|
||
| } | ||
|
|
||
| def consume(consumer: KafkaConsumer[Array[Byte], Array[Byte]], topics: List[String], count: Long, timeout: Long, config: ConsumerPerfConfig, totalMessagesRead: AtomicLong, totalBytesRead: AtomicLong) { | ||
| def consume(consumer: KafkaConsumer[Array[Byte], Array[Byte]], topics: List[String], count: Long, timeout: Long, config: ConsumerPerfConfig, totalMessagesRead: AtomicLong, totalBytesRead: AtomicLong, joinTime: AtomicLong) { | ||
| var bytesRead = 0L | ||
| var messagesRead = 0L | ||
| var lastBytesRead = 0L | ||
| var lastMessagesRead = 0L | ||
| var joinStart = 0L | ||
|
|
||
| // Wait for group join, metadata fetch, etc | ||
| val joinTimeout = 10000 | ||
| val isAssigned = new AtomicBoolean(false) | ||
| consumer.subscribe(topics.asJava, new ConsumerRebalanceListener { | ||
| def onPartitionsAssigned(partitions: util.Collection[TopicPartition]) { | ||
| isAssigned.set(true) | ||
| joinTime.addAndGet(System.currentTimeMillis - joinStart) | ||
| } | ||
| def onPartitionsRevoked(partitions: util.Collection[TopicPartition]) { | ||
| isAssigned.set(false) | ||
| joinStart = System.currentTimeMillis | ||
| }}) | ||
| val joinStart = System.currentTimeMillis() | ||
| val joinStartForFirstTime = System.currentTimeMillis() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we are now reporting the join time separately, I don't think we need the loop below anymore. It was initially intended to discount the rebalance overhead, but now we account for that transparently. |
||
| while (!isAssigned.get()) { | ||
| if (System.currentTimeMillis() - joinStart >= joinTimeout) { | ||
| if (System.currentTimeMillis() - joinStartForFirstTime >= joinTimeout) { | ||
| throw new Exception("Timed out waiting for initial group join.") | ||
| } | ||
| consumer.poll(100) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There doesn't appear to be any reason for
fetchTimeInMsto be anAtomicLong. We can use a regular long and just initialize it below.