lhotari commented on issue #23845:
URL: https://github.com/apache/pulsar/issues/23845#issuecomment-3273436200

   > [@lhotari](https://github.com/lhotari) I did local tests with the fix I 
proposed and the issue is gone. Will do extended tests tomorrow and if they 
show good results I will submit PR by the end of the week.
   
   @nborisov Looking forward to the fix. You can always ping me if you are 
looking for help in getting the tests added. That could be a problem with such 
test scenarios. Sharing some hints, just in case it helps you:
   
   For an end-to-end test for reproducing the issue, one possible test to look 
at is 
[org.apache.pulsar.client.api.KeySharedSubscriptionTest#testOrderingAfterReconnects](https://github.com/apache/pulsar/blob/master/pulsar-broker/src/test/java/org/apache/pulsar/client/api/KeySharedSubscriptionTest.java#L2214)
 or 
[org.apache.pulsar.client.api.KeySharedSubscriptionTest#testDeliveryOfRemainingMessagesWithoutDeadlock](https://github.com/apache/pulsar/blob/13e0a7b89c64bb516d66a48b0da5ede3cff0d8e2/pulsar-broker/src/test/java/org/apache/pulsar/client/api/KeySharedSubscriptionTest.java#L2372).
 Those are Pulsar default type of tests that are technically integration tests 
that use an in memory mocked ZooKeeper and BookKeeper (most of Pulsar broker 
tests are such).
   
   Unit testing of DrainingHashesTracker is simple, but I guess here, the 
interaction between the various classes needs to be tested too. The current 
design isn't well aligned with that, you so might find it painful to add tests.
   
   btw. To test with mocks at some limited level there's an example at 
org.apache.pulsar.broker.service.persistent.PersistentStickyKeyDispatcherMultipleConsumersTest,
 however that example is problematic and injecting race conditions and 
controlling async execution is hard. For #24700, I have a WIP in 
https://github.com/lhotari/pulsar/blob/lh-fix-readMoreEntries-sendInProgress-race-condition-test/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersMockTest.java#L305
 where I was trying to reproduce the issue without success (it seems that the 
race condition is not present).


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to