AndrewJSchofield commented on code in PR #21491:
URL: https://github.com/apache/kafka/pull/21491#discussion_r2824203304
##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -1951,7 +1947,7 @@ private int acquireSubsetBatchRecords(
// records in a single response batch. Condition below checks
if the current record has reached
// the delivery limit and already have some records to return
in response then skip processing
// the current record, which shall be delivered alone in next
fetch.
- if (maxDeliveryCount > 2 && recordDeliveryCount ==
maxDeliveryCount - 1 && acquiredCount > 0) {
+ if (maxDeliveryCount() > 2 && recordDeliveryCount ==
maxDeliveryCount() - 1 && acquiredCount > 0) {
Review Comment:
I'd prefer you to obtain the values of `maxDeliveryCount()` and
`throttleRecordsDeliveryLimit()` at the start of the method and then use those
values throughout. There is a tiny chance that the configs change during the
execution of the method and I think there's a minuscule chance of a peculiar
bug emerging.
##########
core/src/test/scala/unit/kafka/server/KafkaApisTest.scala:
##########
@@ -359,6 +359,7 @@ class KafkaApisTest extends Logging {
cgConfigs.put(SHARE_SESSION_TIMEOUT_MS_CONFIG,
GroupCoordinatorConfig.SHARE_GROUP_SESSION_TIMEOUT_MS_DEFAULT.toString)
cgConfigs.put(SHARE_HEARTBEAT_INTERVAL_MS_CONFIG,
GroupCoordinatorConfig.SHARE_GROUP_HEARTBEAT_INTERVAL_MS_DEFAULT.toString)
cgConfigs.put(SHARE_RECORD_LOCK_DURATION_MS_CONFIG,
ShareGroupConfig.SHARE_GROUP_RECORD_LOCK_DURATION_MS_DEFAULT.toString)
+ cgConfigs.put(SHARE_DELIVERY_COUNT_LIMIT_CONFIG,
ShareGroupConfig.SHARE_GROUP_MAX_DELIVERY_COUNT_LIMIT_DEFAULT.toString)
Review Comment:
I think `ShareGroupConfig.SHARE_GROUP_MAX_DELIVERY_COUNT_LIMIT_DEFAULT`
should be `ShareGroupConfig.SHARE_GROUP_DELIVERY_COUNT_LIMIT_DEFAULT` here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
