Re: [PR] KAFKA-15917: Wait for zombie sink tasks' consumers to commit offsets before trying to modify their offsets in integration tests [kafka]

2024-07-03 Thread via GitHub


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]

2024-07-03 Thread via GitHub


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]

2024-07-02 Thread via GitHub


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]

2024-07-02 Thread via GitHub


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]

2024-07-02 Thread via GitHub


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]

2024-07-02 Thread via GitHub


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