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 2020/12/07 05:04:38 UTC

[GitHub] [kafka] chia7712 commented on a change in pull request #9698: KAFKA-10811: Correct the MirrorConnectorsIntegrationTest to correctly mask the exit procedures

chia7712 commented on a change in pull request #9698:
URL: https://github.com/apache/kafka/pull/9698#discussion_r537233024



##########
File path: connect/mirror/src/test/java/org/apache/kafka/connect/mirror/MirrorConnectorsIntegrationTest.java
##########
@@ -194,20 +224,27 @@ private void waitUntilMirrorMakerIsRunning(EmbeddedConnectCluster connectCluster
 
     @After
     public void close() {
-        for (String x : primary.connectors()) {
-            primary.deleteConnector(x);
-        }
-        for (String x : backup.connectors()) {
-            backup.deleteConnector(x);
-        }
-        deleteAllTopics(primary.kafka());
-        deleteAllTopics(backup.kafka());
-        primary.stop();
-        backup.stop();
         try {
-            assertFalse(exited.get());
+            for (String x : primary.connectors()) {
+                primary.deleteConnector(x);
+            }
+            for (String x : backup.connectors()) {
+                backup.deleteConnector(x);
+            }
+            deleteAllTopics(primary.kafka());
+            deleteAllTopics(backup.kafka());
         } finally {
-            Exit.resetExitProcedure();
+            shuttingDown = true;
+            try {
+                try {
+                    primary.stop();

Review comment:
       How about using ```Utils.closeQuietly``` to replace this nested try-block?

##########
File path: connect/mirror/src/test/java/org/apache/kafka/connect/mirror/MirrorConnectorsIntegrationTest.java
##########
@@ -75,14 +74,45 @@
     private static final int RECORD_CONSUME_DURATION_MS = 20_000;
     private static final int OFFSET_SYNC_DURATION_MS = 30_000;
 
-    private final AtomicBoolean exited = new AtomicBoolean(false);
+    private volatile boolean shuttingDown;
     private Map<String, String> mm2Props;
     private MirrorMakerConfig mm2Config;
     private EmbeddedConnectCluster primary;
     private EmbeddedConnectCluster backup;
 
+    private Exit.Procedure exitProcedure;

Review comment:
       It seems ```exitProcedure``` and ```haltProcedure``` can be local variable




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

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