-
Notifications
You must be signed in to change notification settings - Fork 111
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
Docs: Add recommendations about Session usage. #914
Docs: Add recommendations about Session usage. #914
Conversation
docs/source/connecting/connecting.md
Outdated
@@ -35,6 +35,12 @@ async fn main() -> Result<(), Box<dyn Error>> { | |||
After successfully connecting to some specified node the driver will fetch topology information about | |||
other nodes in this cluster and connect to them as well. | |||
|
|||
Driver maintains this pool of connections and each connection is capable of handling multiple requests in parallel. Driver will also route the request to node / shard that actually owns the data (unless load balancing policy that you use doesn't support it). | |||
For those reasons, we recommend to use one instance of `Session` per application. | |||
Creating short-lived `Session`'s (e.g. `Session` per request) is extremely discouraged and will result in great performance penalties because creating a `Session` is a costly process - it requires estabilishing a lot of TCP connections. |
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.
IMO "extremely" is too strong of a word here. There are cases where short sessions make sense, e.g. an administrative tool that connects only to execute a single statement. Of course, creating multiple sessions over the course of the application is wasteful.
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.
strongly?
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.
I change to strongly
, if that's also too strong I'll rewrite this sentence differently
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.
"Strongly" sounds better to me as well. I was mostly concerned about the "short-lived" sessions given the use cases for them I mentioned in my previous comment, but with the added warning about using only one session per application I think it looks OK now.
2fc8656
to
95330ed
Compare
v3: Fixed typo in preprocessor script |
This will make it easier to add new functionalities to it.
85cac1f
to
f359ed1
Compare
#216 showed that best practices of
Session
usage are not obvious and should be stated in documentation.This PR adds the relevant section that explains how to use it in order to not decrease performance.
Fixes: #216
Pre-review checklist
./docs/source/
.Fixes:
annotations to PR description.