You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/11/08 01:25:22 UTC

[GitHub] [kafka] gharris1727 opened a new pull request, #12830: KAFKA-8115: Reduce flakiness in Trogdor JsonRestServer shutdown

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

   Signed-off-by: Greg Harris <gr...@aiven.io>
   
   The GRACEFUL_SHUTDOWN_TIMEOUT_MS for the Trogdor JsonRestServer is 100ms. In heavily loaded CI environments, this timeout can be exceeded. When this happens, it causes the jettyServer.stop() and jettyServer.destroy() calls to throw exceptions, which prevents shutdownExecutor.shutdown() from running. This has the effect of causing the JsonRestServer::waitForShutdown method to block for 1 day, which exceeds the 120s timeout on the CoordinatorTest (and any other test relying on MiniTrogdorCluster).
   
   This change makes it such that the graceful shutdown timeout is less likely to be exceeded, and when it is, the timeout does not cause the waitForShutdown method to block for much longer than the graceful shutdown timeout.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


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


[GitHub] [kafka] ijuma commented on pull request #12830: KAFKA-8115: Reduce flakiness in Trogdor JsonRestServer shutdown

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #12830:
URL: https://github.com/apache/kafka/pull/12830#issuecomment-1509935873

   Failures seem to be related to `MirrorConnectors` only. There is a separate PR trying to fix that.
   
   > Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationSSLTest.testReplication()
   Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
   Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testCreatePartitionsUseProvidedForwardingAdmin()
   Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testSyncTopicConfigUseProvidedForwardingAdmin()
   Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
   Build / JDK 17 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testSyncTopicConfigUseProvidedForwardingAdmin()
   Build / JDK 17 and Scala 2.13 / kafka.admin.TopicCommandIntegrationTest.testDescribeUnderReplicatedPartitions(String).quorum=kraft
   Build / JDK 17 and Scala 2.13 / org.apache.kafka.tools.MetadataQuorumCommandTest.[1] Type=Raft-Combined, Name=testDescribeQuorumReplicationSuccessful, MetadataVersion=3.5-IV1, Security=PLAINTEXT
   Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
   Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testCreatePartitionsUseProvidedForwardingAdmin()
   Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testSyncTopicConfigUseProvidedForwardingAdmin()
   Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
   Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testCreatePartitionsUseProvidedForwardingAdmin()
   Build / JDK 11 and Scala 2.13 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testSyncTopicConfigUseProvidedForwardingAdmin()
   Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
   Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testCreatePartitionsUseProvidedForwardingAdmin()
   Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testSyncTopicConfigUseProvidedForwardingAdmin()
   Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testReplicationIsCreatingTopicsUsingProvidedForwardingAdmin()
   Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testCreatePartitionsUseProvidedForwardingAdmin()
   Build / JDK 8 and Scala 2.12 / org.apache.kafka.connect.mirror.integration.MirrorConnectorsWithCustomForwardingAdminIntegrationTest.testSyncTopicConfigUseProvidedForwardingAdmin()


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


[GitHub] [kafka] ijuma merged pull request #12830: KAFKA-8115: Reduce flakiness in Trogdor JsonRestServer shutdown

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


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


[GitHub] [kafka] gharris1727 commented on pull request #12830: KAFKA-8115: Reduce flakiness in Trogdor JsonRestServer shutdown

Posted by GitBox <gi...@apache.org>.
gharris1727 commented on PR #12830:
URL: https://github.com/apache/kafka/pull/12830#issuecomment-1324049223

   @stanislavkozlovski @lbradstreet @Kvicii do you think you could help review this test stabilization 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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] ijuma commented on a diff in pull request #12830: KAFKA-8115: Reduce flakiness in Trogdor JsonRestServer shutdown

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


##########
trogdor/src/main/java/org/apache/kafka/trogdor/rest/JsonRestServer.java:
##########
@@ -138,13 +138,13 @@ public void beginShutdown() {
                     log.info("Stopping REST server");
                     jettyServer.stop();
                     jettyServer.join();
+                    jettyServer.destroy();
                     log.info("REST server stopped");
                 } catch (Exception e) {
                     log.error("Unable to stop REST server", e);
                 } finally {
-                    jettyServer.destroy();
+                    shutdownExecutor.shutdown();

Review Comment:
   Shouldn't jetty.destroy remain in the finally and maybe we catch any exceptions it throws?



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


[GitHub] [kafka] ijuma commented on pull request #12830: KAFKA-8115: Reduce flakiness in Trogdor JsonRestServer shutdown

Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #12830:
URL: https://github.com/apache/kafka/pull/12830#issuecomment-1509969743

   Merged to trunk and 3.5.


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