void-ptr974 commented on PR #26146: URL: https://github.com/apache/pulsar/pull/26146#issuecomment-5076478238
Thanks for the questions. The relevant case is not F1 completing with valid ownership. When F1 has a valid owner, callers already holding F1 can return it as soon as F1 completes; this change does not alter that path. The conditional removal only controls the map entry and does not delay or change the result held by F1. The race is in `handleFreeEvent`, where F1 completes with `null`: | Time | Action | `getOwnerRequests` | |---|---|---| | T1 | A lookup is waiting for the current state transition | F1 | | T2 | `handleFreeEvent` removes F1 and completes it with `null` | empty | | T3 | A concurrent lookup or the next assignment installs F2 to wait for the next `Owned` event | F2 | | T4 | The cleanup registered by F1 runs | should remain F2 | | T5 | The next `Owned` event completes F2 | empty | At T4, `remove(serviceUnit)` does not return an owner to F2. It only removes the current map value, which is F2. F2 remains incomplete; when the `Owned` handler runs at T5, it cannot find F2, so the waiting request reaches only the timeout fallback. `remove(serviceUnit, F1)` does not remove F2. The removal at T2 establishes the boundary between the completed request for the previous transition and a request for the next assignment. It must happen before `complete(null)`: completion callbacks can run inline, and a concurrent lookup can run after F1 has been detached. If F1 remained in the map, `dedupeGetOwnerRequest` could return the completed-null F1 instead of creating F2 for the new assignment. F1 cleanup still covers timeout and error completion paths; the conditional removal prevents it from deleting a newer request. -- 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]
