lhotari commented on code in PR #4713:
URL: https://github.com/apache/bookkeeper/pull/4713#discussion_r2891534521
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingWriteLacOp.java:
##########
@@ -86,24 +87,26 @@ void initiate(ByteBufList toSend) {
}
@Override
- public void writeLacComplete(int rc, long ledgerId, BookieId addr, Object
ctx) {
+ public synchronized void writeLacComplete(int rc, long ledgerId, BookieId
addr, Object ctx) {
int bookieIndex = (Integer) ctx;
+ // We got response.
+ receivedResponseSet.clear(bookieIndex);
+
if (completed) {
+ maybeRecycle();
return;
}
if (BKException.Code.OK != rc) {
lastSeenError = rc;
}
- // We got response.
- receivedResponseSet.clear(bookieIndex);
-
if (rc == BKException.Code.OK) {
if (ackSet.completeBookieAndCheck(bookieIndex) && !completed) {
completed = true;
cb.addLacComplete(rc, lh, ctx);
Review Comment:
Is it necessary to execute callbacks outside of the synchronized block?
Perhaps not in this case. In general it's a recommended pattern to avoid dead
locks by limiting the scope of synchronization when possible so that outgoing
calls don't end up causing dead locks by locking other resources. In this case
there's probably not other threads that could hold those required locks while
it calls this writeLacComplete method and would cause a deadlock.
--
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]
