Lari Hotari
Lari Hotari
> After a deep investigation, we found the cause of the error。The root cause due to `sendAddSuccessCallbacks` may be multiple called at the same time. One is that `unsetSuccessAndSendWriteRequest` is...
> At first I was thinking the same thing as you, it didn't seem like it should be happening. But in fact, these two threads can call two different `PendingAddOp`...
@graysonzeng Another thread safety problem is the `changingEnsemble` field. It's modified under the `metadataLock` object monitor. https://github.com/apache/bookkeeper/blob/13e7efaa971cd3613b065ac50836c5ee98985d13/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java#L1895-L1908 should we make `changingEmsemble` field volatile to fix this part of the problem?
> Therefore I want to use sendingCallbacks instead of synchronized Makes sense. The only concern is about correctness. If the method is already processing while it gets called another time,...
Btw. The thread execution model for LedgerHandle isn't consistent. In some cases, the thread is switched to use the ordered executor and sometimes it's not. For example, here the thread...
This observation isn't about this PR directly. I have so many doubts of the correctness of sendAddSuccessCallbacks when it's not executed by the ordered executor. Talking about the current code...
The logic in `sendAddSuccessCallbacks` in master branch feels wrong. The logic gets stuck if the call to sendAddSuccessCallbacks is missed or out of order. Calling the method multiple times won't...
> The sendAddSuccessCallbacks method has no input parameters, which means that assuming both threads want to call it, the execution result will be the same no matter which thread triggers...
> And then sendAddSuccessCallbacks could be made a synchronized method. That would prevent the dead lock seen in [#4171 (comment)](https://github.com/apache/bookkeeper/pull/4171#issuecomment-1886240623) . Makes sense? It's possible that it doesn't solve the...
@graysonzeng I have a question about the problem description: > After a deep investigation, we found the cause of the error。The root cause due to sendAddSuccessCallbacks may be multiple called...