Cole-Greer commented on code in PR #3139:
URL: https://github.com/apache/tinkerpop/pull/3139#discussion_r2159628397
##########
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:
The mechanics of that sound a bit strange to me, but I recall that has all
been subject to considerable deliberation and testing in the past so I won't
weigh in on it here. I agree that given those mechanics, it makes the most
sense to use the client's `hostConnectionPools` to verify availability to avoid
any influence from other clients on the same cluster.
--
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]
