spmallette commented on code in PR #3139:
URL: https://github.com/apache/tinkerpop/pull/3139#discussion_r2159605890
##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java:
##########
@@ -543,9 +543,20 @@ protected void initializeImplementation() {
this.initializationFailure = ex;
}
- // throw an error if there is no host available after initializing
connection pool.
- if (cluster.availableHosts().isEmpty())
+ // throw an error if there is no host available after initializing
connection pool. we used
+ // to test cluster.availableHosts().isEmpty() but checking if we
actually have hosts in
+ // the connection pool seems a bit more fireproof. if we look at
initializeConnectionSetupForHost
+ // we can see that a successful initialization of the
host/connection pool pair is followed by
+ // marking the host available and notifying the load balancing
strategy. by relying directly on
+ // the state of hostConnectionPools we ensure that there is
actually a concrete
+ // host/connection pool pair. even if the connection pool has
immediate problems, it can fallback
+ // to its normal reconnection operation and won't put
chooseConnection in a state where it can
+ // get a NPE if hostConnectionPools ends up being empty. it seems
as if the safest minimum
+ // requirement for leaving this method is to ensure that at least
one ConnectionPool constructor
+ // completed for at least one Host.
+ if (hostConnectionPools.isEmpty()) {
Review Comment:
they never was consistent as best i can tell. the `Host` availability is
`Cluster` wide so it's quite possible that you could have had one `Client` with
an active `Host` then try to spin up a new one and it fail to connect because
the server is down but not mark them unavailable. We'd wait for the original
`Client` to mark it - note the test i modified.
--
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]
