mirror of https://github.com/apache/kafka.git
				
				
				
			MINOR: ensure KafkaServerTestHarness::tearDown is always invoked (#15996)
An exception thrown while closing the client instances in `IntegrationTestHarness::tearDown` may result in `KafkaServerTestHarness::tearDown` not being invoked. This would result in thread leaks of the broker and controller threads spawned in the failing test. An example of this is the [CI run](https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-15994/1/tests) for #15994 where `Build / JDK 8 and Scala 2.12 / testCoordinatorFailover(String, String).quorum=kraft+kip848.groupProtocol=consumer – kafka.api.PlaintextConsumerTest` failing results in `consumers.foreach(_.close(Duration.ZERO))` in `IntegrationTestHarness::tearDown` throwing an exception. A side effect of this is it poisons Gradle test runner JVM and prevents tests in other unrelated classes from executing as `@BeforeAll` check in QuorumTestHarness would cause them to fail immediately. This PR encloses the client closure in try-finally to ensure`KafkaServerTestHarness::tearDown` is always invoked. Reviewers: Nikhil Ramakrishnan <nikrmk@amazon.com>, Igor Soarez <soarez@apple.com>, Chia-Ping Tsai <chia7712@gmail.com>
This commit is contained in:
		
							parent
							
								
									81e6098021
								
							
						
					
					
						commit
						95adb7bfbf
					
				|  | @ -220,16 +220,18 @@ abstract class IntegrationTestHarness extends KafkaServerTestHarness { | |||
| 
 | ||||
|   @AfterEach | ||||
|   override def tearDown(): Unit = { | ||||
|     producers.foreach(_.close(Duration.ZERO)) | ||||
|     consumers.foreach(_.wakeup()) | ||||
|     consumers.foreach(_.close(Duration.ZERO)) | ||||
|     adminClients.foreach(_.close(Duration.ZERO)) | ||||
|     try { | ||||
|       producers.foreach(_.close(Duration.ZERO)) | ||||
|       consumers.foreach(_.wakeup()) | ||||
|       consumers.foreach(_.close(Duration.ZERO)) | ||||
|       adminClients.foreach(_.close(Duration.ZERO)) | ||||
| 
 | ||||
|     producers.clear() | ||||
|     consumers.clear() | ||||
|     adminClients.clear() | ||||
| 
 | ||||
|     super.tearDown() | ||||
|       producers.clear() | ||||
|       consumers.clear() | ||||
|       adminClients.clear() | ||||
|     } finally { | ||||
|       super.tearDown() | ||||
|     } | ||||
|   } | ||||
| 
 | ||||
| } | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue