You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/06/10 23:43:15 UTC
[GitHub] [tinkerpop] simonz-bq opened a new pull request, #1697: Fix potential SessionedClient NPE
simonz-bq opened a new pull request, #1697:
URL: https://github.com/apache/tinkerpop/pull/1697
Potentially fixes https://issues.apache.org/jira/browse/TINKERPOP-2734
Could not reproduce the issue without given reproduction steps, and also seems that it would require specific server circumstance.
However, investigation of the log traces the reporter included in the ticket gives clues of where to look, and a potential but unlikely NPE was logically deduced. Unsure if this will fix this specific user's problem but worth fixing regardless.
As a side effect, also reduces an unneeded coupling due to inheritance on the Client type.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1697: TINKERPOP-2734 Fix potential SessionedClient NPE
Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1697:
URL: https://github.com/apache/tinkerpop/pull/1697#issuecomment-1160721290
sounds good @simonz-bq - that's what i saw too. i could envision where you had a multiple hosts and the session happened to pick an unavailable one at random and then separately a different process could find an available one so that by the time the first session reach the centralized error check it would find an available host. Could you please amend the commit message to include the description of this potentiality? Also, please add a CHANGELOG entry. I'd say this is good to merge following those two things.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1697: TINKERPOP-2734 Fix potential SessionedClient NPE
Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1697:
URL: https://github.com/apache/tinkerpop/pull/1697#issuecomment-1160459458
generally speaking, i don't really see anything wrong with this change. i dont' think it introduces anything bad and the decoupling of logic is probably a good idea. could we question the theory of this change a bit though? i suppose the assumption here that `connectionPool` is somehow `null` when it gets to `SessionedClient.chooseConnection()` and that is the NPE. if so, what circumstance allows that to happen and how does this change either potentially fix or better expose that problem?
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] alexey-temnikov commented on pull request #1697: TINKERPOP-2734 Fix potential SessionedClient NPE
Posted by GitBox <gi...@apache.org>.
alexey-temnikov commented on PR #1697:
URL: https://github.com/apache/tinkerpop/pull/1697#issuecomment-1160552531
@spmallette, in the reported error logs the cause exception stack trace got swallowed - `Could not initialize client for Optional[..]` which may indicate the connection pool hasn't been initialized in `SessionedClient.initializeImplementation()`. By rethrowing the error and decoupling the code instead of NPE we should be able to see a proper underlying stack trace which could lead to the root cause.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1697: TINKERPOP-2734 Fix potential SessionedClient NPE
Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1697:
URL: https://github.com/apache/tinkerpop/pull/1697#issuecomment-1160660043
> which may indicate the connection pool hasn't been initialized in SessionedClient.initializeImplementation()
I understand what it is doing, but I'm more asking, is there a theory as to why it is not getting initialized?
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette merged pull request #1697: TINKERPOP-2734 Fix potential SessionedClient NPE
Posted by GitBox <gi...@apache.org>.
spmallette merged PR #1697:
URL: https://github.com/apache/tinkerpop/pull/1697
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] simonz-bq commented on pull request #1697: TINKERPOP-2734 Fix potential SessionedClient NPE
Posted by GitBox <gi...@apache.org>.
simonz-bq commented on PR #1697:
URL: https://github.com/apache/tinkerpop/pull/1697#issuecomment-1160709486
> if so, what circumstance allows that to happen and how does this change either potentially fix or better expose that problem?
In the case that initializing the connection pool had a failure *but* somehow the cluster has available hosts `cluster.availableHosts().isEmpty()` in the list (not necssarily alive), there wouldn't be a connection pool initialized but an error would not be throw because of this check being possibly false.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org