lhotari commented on PR #4171:
URL: https://github.com/apache/bookkeeper/pull/4171#issuecomment-1888561697
> 1. So we not use the OrderedExecutor, and should synchronized
(pendingAddOps) for serializing drainPendingAddsAndAdjustLength &
sendAddSuccessCallbacks. Is it right? @lhotari
I don't think that using OrderedExecutor is justified since callback
responses have never been executed by the OrderedExecutor. All
sendAddSuccessCallbacks & drainPendingAddsAndAdjustLength calls should be
serialized for correctness and that's what `synchronized (pendingAddOps) {`
solves without causing a risk for a dead lock.
> remove the too strict rule for pendingAddsSequenceHead which breaks things
after failures
>
> I'm a little confused about this . Can you tell it more about it? @lhotari
This rule 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
My assumption is that this doesn't hold under failure conditions. Why would
there even need to be such a rule?
When this rule kicks in, the LedgerHandle will stop calling callbacks. That
doesn't make any sense to me.
Here's a sign that the entry id isn't always a continuous sequence:
In LedgerHandle.updateLastConfirmed:
https://github.com/apache/bookkeeper/blob/13e7efaa971cd3613b065ac50836c5ee98985d13/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java#L1372
When the LAC jumps forward and `lastAddPushed = Math.max(lastAddPushed,
lac)` kicks in, the pendingAddsSequenceHead rule would again prevent
LedgerHandle from calling callbacks in `sendAddSuccessCallbacks`.
I wonder if I'm making correct observations about the code. @eolivelli could
you please explain how the above condition is handled?
--
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]
