-
Notifications
You must be signed in to change notification settings - Fork 994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PHOENIX-6276: Log when hconnection is getting closed in ConnectionQueryServicesImpl #1057
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably better to wrap the HConnection close operation in a method similar to openConnection so that we don't have to duplicate the logging logic each time (and perhaps miss a case)
eadee0c
to
62e4dde
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 assuming a good test run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change requested, otherwise +1
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
Show resolved
Hide resolved
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
Show resolved
Hide resolved
62e4dde
to
870bbf1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 if preCommit passes. Thanks @sandeepvinayak
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
870bbf1
to
61f9c12
Compare
💔 -1 overall
This message was automatically generated. |
61f9c12
to
b834ca3
Compare
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
Outdated
Show resolved
Hide resolved
b834ca3
to
d703d50
Compare
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 (non-binding)
Thanks for the patch, @sandeepvinayak. Committing. |
No description provided.