You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "dajac (via GitHub)" <gi...@apache.org> on 2026/01/19 18:09:17 UTC

[PR] MINOR: Simplify timeout handling in CoordinatorRuntime [kafka]

dajac opened a new pull request, #21334:
URL: https://github.com/apache/kafka/pull/21334

   Remove the timeout parameter from all CoordinatorRuntime schedule methods and use the writeTimeout field (configured via the builder) instead. This simplifies the API since the timeout is now configured once at runtime construction time.
   
   - Rename defaultWriteTimeout to writeTimeout
   - Rename withDefaultWriteTimeOut to withWriteTimeout
   - Remove timeout parameter from scheduleWriteOperation, scheduleWriteAllOperation, scheduleTransactionalWriteOperation, and scheduleTransactionCompletion methods
   - Update GroupCoordinator.completeTransaction to remove timeout parameter
   - Update all callers and tests accordingly
   


-- 
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


Re: [PR] MINOR: Simplify timeout handling in CoordinatorRuntime [kafka]

Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on code in PR #21334:
URL: https://github.com/apache/kafka/pull/21334#discussion_r2712872115


##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -1792,8 +1791,7 @@ class KafkaApis(val requestChannel: RequestChannel,
               marker.producerEpoch,
               marker.coordinatorEpoch,
               marker.transactionResult,
-              markerTransactionVersion,
-              Duration.ofMillis(config.requestTimeoutMs.toLong)

Review Comment:
   With this change the timeout is reduced from a default of 30 seconds to a default of 5 seconds. I have no objection to this.
   
   We become inconsistent with the non-`__consumer_offsets` WriteTxnMarkers timeout below but I think it's worth the simplicity.



-- 
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


Re: [PR] MINOR: Simplify timeout handling in CoordinatorRuntime [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #21334:
URL: https://github.com/apache/kafka/pull/21334


-- 
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


Re: [PR] MINOR: Simplify timeout handling in CoordinatorRuntime [kafka]

Posted by "squah-confluent (via GitHub)" <gi...@apache.org>.
squah-confluent commented on code in PR #21334:
URL: https://github.com/apache/kafka/pull/21334#discussion_r2712787075


##########
coordinator-common/src/main/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntime.java:
##########
@@ -2103,7 +2102,7 @@ public void onHighWatermarkUpdated(
      * @param coordinatorShardBuilderSupplier   The coordinator builder.
      * @param time                              The system time.
      * @param timer                             The system timer.
-     * @param defaultWriteTimeout               The write operation timeout.
+     * @param writeTimeout               The write operation timeout.

Review Comment:
   nit: alignment
   ```suggestion
        * @param writeTimeout                      The write operation timeout.
   ```



-- 
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


Re: [PR] MINOR: Simplify timeout handling in CoordinatorRuntime [kafka]

Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on code in PR #21334:
URL: https://github.com/apache/kafka/pull/21334#discussion_r2712601768


##########
core/src/main/scala/kafka/server/KafkaApis.scala:
##########
@@ -1792,8 +1791,7 @@ class KafkaApis(val requestChannel: RequestChannel,
               marker.producerEpoch,
               marker.coordinatorEpoch,
               marker.transactionResult,
-              markerTransactionVersion,
-              Duration.ofMillis(config.requestTimeoutMs.toLong)

Review Comment:
   There is one change worth pointing out. This is the only case were we did not use the default timeout (the offset timeout). However, I don't really a reason to not use it in order to be consistent.



-- 
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