BewareMyPower commented on PR #20595: URL: https://github.com/apache/pulsar/pull/20595#issuecomment-1612380108
I made new changes to avoid race condition described in https://github.com/apache/pulsar/pull/20595#discussion_r1245387433 The root cause is that for a `grabCnx` operation, `duringConnect.set(false)` could be called twice and `reconnectLater` could be called by the connection implementation. So I removed the `reconnectLater` calls in the `Connection#connectionOpened` implementations and just complete the future exceptionally so that `reconnectLater` will be called later. ```java cnxFuture.thenCompose(cnx -> connection.connectionOpened(cnx)) .thenAccept(__ -> duringConnect.set(false)) .exceptionally(this::handleConnectionError); ``` All possible cases are: 1. `cnxFuture` completes with null 1.1 `connectionOpened` completes with null, `thenAccept` will set `duringConnect` with false. 1.2 `connectionOpened` completes exceptionally, `exceptionally` will call `reconnectLater`, which sets `duringConnect` with false. 2. `cnxFuture` completes exceptionally, `exceptionally` will call `reconnectLater`, which sets `duringConnect` with false. Ideally, `reconnectLater` should be private, but it seems hard to remove the `reconnectLater` calls in `TransactionMetaStoreHandler`. PTAL again. @poorbarcode @lifepuzzlefun @tisonkun @Technoboy- -- 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]
