You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "chia7712 (via GitHub)" <gi...@apache.org> on 2023/02/20 14:00:41 UTC
[GitHub] [kafka] chia7712 opened a new pull request, #13279: KAFKA-14295 FetchMessageConversionsPerSec meter not recorded
chia7712 opened a new pull request, #13279:
URL: https://github.com/apache/kafka/pull/13279
https://issues.apache.org/jira/browse/KAFKA-14295
The broker topic metric FetchMessageConversionsPerSec doesn't get recorded on a fetch message conversion.
The bug is that we pass in a callback that expects a MultiRecordsSend in KafkaApis:
```scala
def updateConversionStats(send: Send): Unit = {
send match {
case send: MultiRecordsSend if send.recordConversionStats != null =>
send.recordConversionStats.asScala.toMap.foreach {
case (tp, stats) => updateRecordConversionStats(request, tp, stats)
}
case _ =>
}
}
```
But we call this callback with a NetworkSend in the SocketServer:
```scala
selector.completedSends.forEach { send =>
try {
val response = inflightResponses.remove(send.destinationId).getOrElse {
throw new IllegalStateException(s"Send for ${send.destinationId} completed, but not in `inflightResponses`")
}
updateRequestMetrics(response)
// Invoke send completion callback
response.onComplete.foreach(onComplete => onComplete(send))
```
Note that Selector.completedSends returns a collection of NetworkSend
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a diff in pull request #13279: KAFKA-14295 FetchMessageConversionsPerSec meter not recorded
Posted by "showuon (via GitHub)" <gi...@apache.org>.
showuon commented on code in PR #13279:
URL: https://github.com/apache/kafka/pull/13279#discussion_r1115304004
##########
core/src/test/scala/unit/kafka/server/FetchRequestDownConversionConfigTest.scala:
##########
@@ -221,6 +222,8 @@ class FetchRequestDownConversionConfigTest extends BaseRequestTest {
} else {
assertEquals(Errors.UNSUPPORTED_VERSION, error(partitionWithDownConversionDisabled))
}
+ TestUtils.waitUntilTrue(() => TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec) > initialFetchMessageConversionsPerSec,
+ s"init: $initialFetchMessageConversionsPerSec final: ${TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec)}", 3000)
Review Comment:
nit: The error message could be more clear, ex:
```
The `FetchMessageConversionsPerSec` metric count is not incremented after 3 seconds. init: $initialFetchMessageConversionsPerSec final: ${TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec)}
```
Also, the 3 seconds might be able to increase to 5 sec to avoid flaky failure.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 merged pull request #13279: KAFKA-14295 FetchMessageConversionsPerSec meter not recorded
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #13279:
URL: https://github.com/apache/kafka/pull/13279
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on a diff in pull request #13279: KAFKA-14295 FetchMessageConversionsPerSec meter not recorded
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #13279:
URL: https://github.com/apache/kafka/pull/13279#discussion_r1115438116
##########
core/src/test/scala/unit/kafka/server/FetchRequestDownConversionConfigTest.scala:
##########
@@ -221,6 +222,8 @@ class FetchRequestDownConversionConfigTest extends BaseRequestTest {
} else {
assertEquals(Errors.UNSUPPORTED_VERSION, error(partitionWithDownConversionDisabled))
}
+ TestUtils.waitUntilTrue(() => TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec) > initialFetchMessageConversionsPerSec,
+ s"init: $initialFetchMessageConversionsPerSec final: ${TestUtils.metersCount(BrokerTopicStats.FetchMessageConversionsPerSec)}", 3000)
Review Comment:
thanks for review. will address your comment
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #13279: KAFKA-14295 FetchMessageConversionsPerSec meter not recorded
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #13279:
URL: https://github.com/apache/kafka/pull/13279#issuecomment-1442190318
```
Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
```
```
Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.DedicatedMirrorIntegrationTest.testMultiNodeCluster()
```
unrelated error
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org