Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]
C0urante commented on PR #15302: URL: https://github.com/apache/kafka/pull/15302#issuecomment-2206277714 Thanks Yash! -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]
C0urante merged PR #15302: URL: https://github.com/apache/kafka/pull/15302 -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]
yashmayya commented on code in PR #15302: URL: https://github.com/apache/kafka/pull/15302#discussion_r1663466038 ## connect/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java: ## @@ -976,27 +988,53 @@ private String modifySinkConnectorOffsetsWithRetry(ConnectorOffsets offsetsToAlt * * @param connectorName the name of the sink connector whose offsets are to be verified * @param expectedTopic the name of the Kafka topic that the sink connector is consuming from - * @param expectedPartitions the number of partitions that exist for the Kafka topic + * @param expectedPartitions the number of partitions that exist for the Kafka<<< topic Review Comment: Stray change? -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]
C0urante commented on code in PR #15302: URL: https://github.com/apache/kafka/pull/15302#discussion_r1663319861 ## connect/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java: ## @@ -801,6 +806,11 @@ public void testResetSinkConnectorOffsetsZombieSinkTasks() throws Exception { connect.assertions().assertConnectorAndAtLeastNumTasksAreRunning(connectorName, 1, "Connector tasks did not start in time."); +// Make sure the tasks' consumers have had a chance to actually form a group +// (otherwise, the reset request will succeed because there won't be any active consumers) +verifyExpectedSinkConnectorOffsets(connectorName, topic, 1, NUM_RECORDS_PER_PARTITION, Review Comment: Great point, I've removed mention of KAFKA-15524 from this PR. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]
C0urante commented on PR #15302: URL: https://github.com/apache/kafka/pull/15302#issuecomment-2204763797 @gharris1727 sorry for the delay. You're correct that this PR did not actually address KAFKA-15224, and I'm honestly not sure how I missed that the first time around. I've updated the description to only mention KAFKA-15917, added some useful info to assertion messages when sink connectors fail to catch up to the expected committed offsets in these tests, and rebased on the latest trunk. Would you mind giving this another pass when you have a moment? Currently this test is the flakiest of all Connect integration tests, failing nearly 10% of the time. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]
C0urante commented on code in PR #15302: URL: https://github.com/apache/kafka/pull/15302#discussion_r1663319861 ## connect/runtime/src/test/java/org/apache/kafka/connect/integration/OffsetsApiIntegrationTest.java: ## @@ -801,6 +806,11 @@ public void testResetSinkConnectorOffsetsZombieSinkTasks() throws Exception { connect.assertions().assertConnectorAndAtLeastNumTasksAreRunning(connectorName, 1, "Connector tasks did not start in time."); +// Make sure the tasks' consumers have had a chance to actually form a group +// (otherwise, the reset request will succeed because there won't be any active consumers) +verifyExpectedSinkConnectorOffsets(connectorName, topic, 1, NUM_RECORDS_PER_PARTITION, Review Comment: Great point, I've removed mention of KAFKA-15224 from this PR. -- 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-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org