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

Retry to get session when session is invalid in SessionPool #252

Merged
merged 5 commits into from
Feb 10, 2023

Conversation

Aiee
Copy link
Contributor

@Aiee Aiee commented Feb 8, 2023

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

Close #248

Description:

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

@Aiee Aiee changed the base branch from master to release-3.4-beta-sc February 8, 2023 09:21
@wey-gu
Copy link
Contributor

wey-gu commented Feb 8, 2023

we may need such retry mechanism in all clients :D

session_pool.go Outdated Show resolved Hide resolved
session_pool.go Outdated Show resolved Hide resolved
@Aiee Aiee requested a review from HarrisChu February 10, 2023 07:07
@Aiee Aiee merged commit f92db99 into vesoft-inc:release-3.4-beta-sc Feb 10, 2023
@Aiee Aiee deleted the reconnect-session branch February 10, 2023 10:23
@wenhaocs
Copy link

Can you merge this PR to master? We are still working on the http support for beta-sc, which may or may not be used. In addition, I think this feature is useful for all. Thanks!

Comment on lines +402 to +403
pool.rwLock.Lock()
defer pool.rwLock.Unlock()
Copy link
Contributor

@veezhang veezhang Feb 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be possible to reduce the granularity of lock?
Only need to lock *list.List, not during session creation, execution, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should recheck the lock usage later.

@wenhaocs wenhaocs linked an issue Feb 28, 2023 that may be closed by this pull request
Sophie-Xie pushed a commit that referenced this pull request Mar 3, 2023
* Update readme for v3.4 release (#250)

* Fix format

* Update README

* Retry to get session when session is invalid

* Simplify client test

* Add comments

* Add comments
Aiee added a commit that referenced this pull request Mar 3, 2023
…261)

* Update readme for v3.4 release (#250)

* Fix format

* Update README

* Retry to get session when session is invalid

* Simplify client test

* Add comments

* Add comments

Co-authored-by: Yichen Wang <18348405+Aiee@users.noreply.github.com>
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.

Recreate session in SessionPool on ErrorCode_E_SESSION_INVALID
5 participants