kezhuw opened a new pull request #3041:
URL: https://github.com/apache/bookkeeper/pull/3041


   Descriptions of the changes in this PR:
   
   
   ### Motivation
   Currently, in `LedgerHandle.unsetSuccessAndSendWriteRequest`,
   `LedgerHandle.sendAddSuccessCallbacks` could be called by
   `PendingAddOp.unsetSuccessAndSendWriteRequest` **before** all pending
   adds evaluated. This will make entries which met ack requirement in old
   ensemble but have not evaluated yet succeed in new ensemble.
   
   ### Changes
   Check succeeded entries after unsetting all pending entries in ensemble 
change unsetting.
   
   Master Issue: #3005
   
   ### Further thoughts
   `PendingAddOp.writeComplete` 
[calls](https://github.com/apache/bookkeeper/blob/0de75bc1ad6521bb96e358fe69882425afe8c260/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingAddOp.java#L315)
 `LedgerHandle.sendAddSuccessCallbacks` in `completed` branch.
   
   Currently, `LedgerHandle.sendAddSuccessCallbacks` will not be called if 
failed bookies overlap with all pending write ensembles. So, the code in 
`PendingAddOp.writeComplete` sounds help.
   
   But after changed,  `LedgerHandle.sendAddSuccessCallbacks` will be called 
unconditionally after all pending adds complete unsetting. So I think the 
snippet in `PendingAddOp.writeComplete` does not help anymore. Can it be 
dropped ? I am not that confident. Post my thoughts here for evaluation.


-- 
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