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

Reply via email to