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

CASSGO-3 detailed description for NumConns #1821

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

OleksiienkoMykyta
Copy link

Closes #1741 .
There is some unclearness in the NumConns description because there are no details about what it affects, and does it sets any limits. I added a few more details to avoid confusion about it in the future.

@joao-r-reis joao-r-reis changed the title detailed description for NumConns CASSGO-3 detailed description for NumConns Oct 29, 2024
cluster.go Outdated
@@ -102,7 +102,8 @@ type ClusterConfig struct {
// Initial keyspace. Optional.
Keyspace string

// Number of connections per host.
// The size of connection pool for each host. The Pool will be filled during the first request execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this is 100% correct, if InitialHostLookup is enabled (it's the default) then the pools for the discovered hosts will be filled during session initialization

Copy link
Author

Choose a reason for hiding this comment

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

You are right!
I double-checked it with the timeout, and it turned out that the pool filled during initialization. When I tested it last time the pool had not been filled before the query execution because pool filling runs in separate gorutine and it took more time to finish than the query execution.
Thank you for noticing the issue!
I can replace it with something like

The size of the connection pool for each host. The pool filling runs in separate gourutine during the session initialization phase. 
Also, it describes a maximum number of connections at the same time.
Notice: There is no guarantee that pool filling will be finished in the initialization phase.
Default: 2 

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm interesting, did you use SingleHostReadyPolicy? If you don't use this policy then it should wait until all hosts have at least 1 connection in the pool before the session initialization is done.

Check this and this.

Considering this, it would probably be good to mention that gocql will always try to get 1 connection on each host pool during session initialization AND it will attempt to fill each pool afterwards asynchronously if NumConns > 1

Copy link
Author

Choose a reason for hiding this comment

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

What about something like that?

// The size of the connection pool for each host.
// The pool filling runs in separate gourutine during the session initialization phase.
// gocql will always try to get 1 connection on each host pool
// during session initialization AND it will attempt
// to fill each pool afterward asynchronously if NumConns > 1.
// Notice: There is no guarantee that pool filling will be finished in the initialization phase.
// Also, it describes a maximum number of connections at the same time.

I have added gocql will always try to get 1 connection on each host pool during session initialization AND it will attempt to fill each pool afterward asynchronously if NumConns > 1 as you suggested, so there is no more room for unclearness.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@OleksiienkoMykyta OleksiienkoMykyta force-pushed the issue-1741-cant-limit-number-of-connections-per-pod branch from 9a3f820 to f7bdc05 Compare November 15, 2024 14:09
@OleksiienkoMykyta OleksiienkoMykyta force-pushed the issue-1741-cant-limit-number-of-connections-per-pod branch from f7bdc05 to 8ebc42d Compare November 15, 2024 14:14
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.

CASSGO-3 How to set limit for pool connection
2 participants