nicoloboschi opened a new pull request, #15638: URL: https://github.com/apache/pulsar/pull/15638
### Motivation There are some tests that occasionally fails with non-sense mockito errors like this one: ``` org.apache.pulsar.broker.PulsarServerException: org.mockito.exceptions.misusing.WrongTypeOfReturnValue: BrokerService cannot be returned by getConfiguration() getConfiguration() should return ServiceConfiguration *** If you're unsure why you're getting above error read on. Due to the nature of the syntax above problem might occur because: 1. This exception *might* occur in wrongly written multi-threaded tests. Please refer to Mockito FAQ on limitations of concurrency testing. 2. A spy is stubbed using when(spy.foo()).then() syntax. It is safer to stub spies - - with doReturn|Throw() family of methods. More in javadocs for Mockito.spy() method. at org.apache.pulsar.broker.service.BrokerService.close(BrokerService.java:685) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:710) at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418) Caused by: org.mockito.exceptions.misusing.WrongTypeOfReturnValue: BrokerService cannot be returned by getConfiguration() getConfiguration() should return ServiceConfiguration *** If you're unsure why you're getting above error read on. Due to the nature of the syntax above problem might occur because: 1. This exception *might* occur in wrongly written multi-threaded tests. Please refer to Mockito FAQ on limitations of concurrency testing. 2. A spy is stubbed using when(spy.foo()).then() syntax. It is safer to stub spies - - with doReturn|Throw() family of methods. More in javadocs for Mockito.spy() method. at org.apache.pulsar.broker.PulsarService.getConfiguration(PulsarService.java:608) at org.apache.pulsar.broker.service.BrokerService.shutdownEventLoopGracefully(BrokerService.java:846) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:710) at org.apache.pulsar.broker.service.BrokerService.shutdownEventLoopGracefully(BrokerService.java:846) at org.apache.pulsar.broker.service.BrokerService.closeAsync(BrokerService.java:757) at java.base/java.lang.invoke.MethodHandle.invokeWithArguments(MethodHandle.java:710) at org.apache.pulsar.broker.service.BrokerService.closeAsync(BrokerService.java:720) at org.apache.pulsar.broker.service.BrokerService.close(BrokerService.java:680) ... 2 more ``` After investigating I noticed that MetadataStore threads can still receive notifications even if the current test method is finished. Also other thread pools are not shutdown in the correct way and the test must wait for their terminations in order to avoid mixing up Mockito mocked objects. ### Modifications - Enhanced `ManagedLedgerFactoryImpl#closeAsync` method - PulsarService#close sync method now wait for EventLoopGroup to be terminated - Introduced new method closeAsync for CoordinationService class to enhance parallelism while disposing resources in PulsarService and tests - Fixed a couple of tests which manually create executors and Netty event loop groups This is related to past flaky-tests like: - https://github.com/apache/pulsar/issues/13808 - https://github.com/apache/pulsar/pull/14006 - https://github.com/apache/pulsar/issues/13620 fix #15774 - [x] `no-need-doc` -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org