pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

[fix][ml] Cancel Metadata Store listeners at shutdown to fix resource leaks in some tests

Open lhotari opened this issue 8 months ago • 1 comments

Motivation

The current MetadataStore API and MetadataStoreExtended API doesn't provide a way to cancel (unregister) registered listeners. This is a problem in some tests where the metadata store instance is shared, but the brokers are restarted during the test. Notifications will keep on delivered to the broker instances that have already been shutdown. In addition, to the above problem, the listeners will also keep a strong reference to instances that have already been shutdown, impacting memory usage in some tests. One of the additional motivations is to cleanup the testing infrastructure so that it would be possible to migrate Pulsar to use the most recent Mockito version, 5.17.0. When mocks are invalidated in newer Mockito versions, they will throw exceptions upon execution. This change is made to ensure that these problems that are currently seen in PR #24241 aren't originating from Metadata Store listeners.

Modifications

Add new methods:

  • registerCancellableListener to MetadataStore
  • registerCancellableSessionListener to MetadataStoreExtended

Make the API binary compatible by adding default methods registerListener and registerSessionListener with the previous method signatures so that plugins using the Metadata API won't break due to this change. Mark these methods deprecated.

Migrate the Pulsar code to use the cancellable methods and store the callback instance for cancellation and use this in the shutdown sequence of each component that registers listeners.

Documentation

  • [ ] doc
  • [ ] doc-required
  • [x] doc-not-needed
  • [ ] doc-complete

lhotari avatar May 06 '25 12:05 lhotari

@lhotari This test has failed five times, maybe you should look into it. org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryShutdownTest.openEncounteredShutdown

liangyepianzhou avatar May 07 '25 02:05 liangyepianzhou