Lari Hotari

@graysonzeng After spending more time with this issue, here are my suggestions to fix the issue: * make `sendAddSuccessCallbacks` method `synchronized`. * make `changingEnsemble` field `volatile` * change line 204...

> The `changingEnsemble` is already protected by synchronized, why do we need volatile? @hangc0276 `changingEnsemble` is mutated under `synchronized (metadataLock) {`. A mutation in a synchronized block isn't sufficient alone....

> After we make `sendAddSuccessCallbacks` is called by the same thread, we don't need synchronize. @hangc0276 True, but that would be a significant change that could at least cause performance...

I created an alternative fix #4175 to bring clarity of what I'd be proposing. Once we have a reproducer for the issue in BK as a unit test, it will...

> 1. So we not use the OrderedExecutor, and should synchronized (pendingAddOps) for serializing drainPendingAddsAndAdjustLength & sendAddSuccessCallbacks. Is it right? @lhotari @graysonzeng I don't think that using OrderedExecutor is justified...

> Thanks for your reply @lhotari, I have some test for https://github.com/apache/bookkeeper/pull/4175 , Unfortunately it will also cause deadlock. > Suppose we are not willing to use OrderedExecutor because of...

> Thanks for your help @lhotari ，I have updated the PR and test it again. Can you help me review it again? @hangc0276 @eolivelli I also think OrderedExecutor will cause...

Client's BookieWriteLedgerTest and DeferredSyncTest are failing. It is always possible that those tests are invalid. The logic adding for deferred sync doesn't make sense to me. https://github.com/apache/bookkeeper/blob/13e7efaa971cd3613b065ac50836c5ee98985d13/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java#L1822-L1829 More comments about...

@graysonzeng is this related to #4194 / #4097 ?

A revisited broker entry cache solution is in progress in lhotari/pulsar#209