Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on PR #15613: URL: https://github.com/apache/kafka/pull/15613#issuecomment-2040495611 Thanks for the changes @lucasbru, looks good to me overall. This is tidying up the whole async commit callbacks execution story. Left some comments, mostly minor, and to make sure we're

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554162586 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554177925 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554165141 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554162586 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554162586 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554025678 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554024796 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554024079 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554020736 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinatorTest.java: ## @@ -229,7 +229,11 @@ private GroupRebalanceConfig

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1554011665 ## core/src/test/scala/integration/kafka/api/PlaintextConsumerTest.scala: ## @@ -654,6 +654,64 @@ class PlaintextConsumerTest extends BaseConsumerTest {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543492385 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -984,6 +984,8 @@ public void close(final Timer timer) {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543454136 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -1164,7 +1176,8 @@ public void maybeAutoCommitOffsetsAsync(long

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543454136 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -1164,7 +1176,8 @@ public void maybeAutoCommitOffsetsAsync(long

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543454136 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -1164,7 +1176,8 @@ public void maybeAutoCommitOffsetsAsync(long

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1553899936 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1360,6 +1362,9 @@ public void commitSync(Map offsets, Duration

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-04-05 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1553899936 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1360,6 +1362,9 @@ public void commitSync(Map offsets, Duration

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543492385 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -984,6 +984,8 @@ public void close(final Timer timer) {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543492385 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -984,6 +984,8 @@ public void close(final Timer timer) {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543492385 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -984,6 +984,8 @@ public void close(final Timer timer) {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543492385 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -984,6 +984,8 @@ public void close(final Timer timer) {

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lianetm commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543454136 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -1164,7 +1176,8 @@ public void maybeAutoCommitOffsetsAsync(long

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lucasbru commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543068952 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/AsyncKafkaConsumer.java: ## @@ -1360,6 +1367,9 @@ public void commitSync(Map offsets, Duration

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lucasbru commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543066981 ## clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java: ## @@ -1164,7 +1176,8 @@ public void maybeAutoCommitOffsetsAsync(long

Re: [PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-28 Thread via GitHub
lucasbru commented on code in PR #15613: URL: https://github.com/apache/kafka/pull/15613#discussion_r1543063391 ## clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinatorTest.java: ## @@ -229,7 +229,11 @@ private GroupRebalanceConfig

[PR] KAFKA-16103: commitSync should await pending async commits [kafka]

2024-03-27 Thread via GitHub
lucasbru opened a new pull request, #15613: URL: https://github.com/apache/kafka/pull/15613 The javadoc for `KafkaConsumer.commitSync` says: > Note that asynchronous offset commits sent previously with the {@link #commitAsync(OffsetCommitCallback)} > (or similar) are guaranteed to