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

Add session pool #229

Merged
merged 7 commits into from
Oct 20, 2022
Merged

Add session pool #229

merged 7 commits into from
Oct 20, 2022

Conversation

Aiee
Copy link
Contributor

@Aiee Aiee commented Oct 18, 2022

Why we need a session pool

The purpose of adding the session pool is to solve the problem caused by improperly using the connection pool. For example, a common case is that the user generates a new session, executes a query, and releases the session in a loop, like:

// Wrong usage of the connection pool
for(int i=0; i<100; i++) {
  // get new session
  // execute query
  // release session
}

This will cause huge traffic in the meta service and may crash the service.

Usage

See example/SessinPoolExample.py for a more detailed example.

The usage of the connection pool remains unchanged.

Limitation

There are some limitations:

  1. There MUST be an existing space in the DB before initializing the session pool.
  2. Each session pool is corresponding to a single USER and a single Space. This is to ensure that the user's access control is consistent. i.g. The same user may have different access privileges in different spaces. If you need to run queries in different spaces, you may have multiple session pools.
  3. Every time when sessinPool.execute() is called, the session will execute the query in the space set in the session pool config.
  4. Commands that alter passwords or drop users should NOT be executed via session pool.

@Aiee Aiee requested a review from wey-gu October 19, 2022 11:15
nebula3/gclient/net/SessionPool.py Outdated Show resolved Hide resolved
)
return session
except AuthFailedException as e:
# if auth failed, close the pool
Copy link
Contributor

Choose a reason for hiding this comment

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

What if somehow the authentication failed not due to bad credentials, but because it was something wrong with the metaD? Will closing the pool still be valid in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, I'll add check for the error code to handle the credential case specifically. For non-credential errors, the pool should remain open.

Copy link
Contributor

@wey-gu wey-gu left a comment

Choose a reason for hiding this comment

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

Great Job!!!

@Aiee Aiee merged commit 25eb787 into vesoft-inc:master Oct 20, 2022
@Aiee Aiee deleted the session-pool branch October 20, 2022 06:26
@Sophie-Xie Sophie-Xie mentioned this pull request Oct 21, 2022
@wey-gu wey-gu mentioned this pull request Nov 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.

3 participants