Skip to content
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

[CALCITE-5077] ResetSession implements driver.SessionResetter. #57

Closed
wants to merge 6 commits into from

Conversation

liguozhong
Copy link
Contributor

avatica sql query fails after running for a period of time on our online loki, this PR attempts to solve this "NoSuchConnectionException" problem.

level=error ts=2022-03-31T12:51:09.356233496Z caller=table_manager.go:233 msg="error syncing tables" err="An error was encountered while processing your request: NoSuchConnectionException{connectionId='Connection not found: invalid id, closed, or expired: ccd95a16-5496-7001-f99b-1ab0e5f19e05'}"

@F21
Copy link
Member

F21 commented Mar 31, 2022

Hey thanks for working on this. Can you open an issue at https://issues.apache.org/jira/secure/Dashboard.jspa and change the title of the PR to [CALCITE-XXXX] Title of PR? See the updated title in #56 as an example.

@liguozhong
Copy link
Contributor Author

Hey thanks for working on this. Can you open an issue at https://issues.apache.org/jira/secure/Dashboard.jspa and change the title of the PR to [CALCITE-XXXX] Title of PR? See the updated title in #56 as an example.

Ok, I will go to jira to open an issue, I have no PR experience with apache foundation.
Thanks for your quick response .

@liguozhong liguozhong changed the title [WIP] ResetSession implements driver.SessionResetter. [WIP] [CALCITE-5077] ResetSession implements driver.SessionResetter. Apr 1, 2022
@liguozhong
Copy link
Contributor Author

@F21 done.

this pr has fixed "NoSuchConnectionException" issue in my loki cluster. and "NoSuchConnectionException" no longer appears in the log, but I'm not a database expert, @F21 I still need your help to review the code .

image

without this pr
image

this this pr
image

@F21
Copy link
Member

F21 commented Apr 1, 2022

Looks good to me! Please squash your commits and make the commit message the same as the title of the PR.

@liguozhong liguozhong changed the title [WIP] [CALCITE-5077] ResetSession implements driver.SessionResetter. [CALCITE-5077] ResetSession implements driver.SessionResetter. Apr 1, 2022
ResetSession implements driver.SessionResetter.

reuse connectionId.

remove unuse field.
@F21
Copy link
Member

F21 commented Apr 1, 2022

You can squash the commits by following these steps: https://gitbetter.substack.com/p/how-to-squash-git-commits?s=r

@liguozhong
Copy link
Contributor Author

Looks good to me! Please squash your commits and make the commit message the same as the title of the PR.

#59
@F21 Hi, I have some problems with rebase, I submitted a new PR(#59), can I close this PR by merging another PR, I am not good at using rebase to merge multiple commits.thanks

@F21
Copy link
Member

F21 commented Apr 1, 2022

@liguozhong That's no problem. Let's move the conversation to #59

@liguozhong
Copy link
Contributor Author

thanks for your help

@liguozhong liguozhong closed this Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants