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

Implement Connection Pooling and Threading #91

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

MrLight
Copy link
Contributor

@MrLight MrLight commented Oct 6, 2024

This implements Connection Pooling for the Snowflake Connection.
The code today doesn't reuse the snowflake connection. After each query the connection is dropped. This PullRequest will add ConnectionPooling to store open connections for later reuse. The poolsize and the connection timeout can be configured in the PluginConfig.
When the snowflake warehouse is in hard load it could be possible, that too much queries will be forwarded to snowflake, which will end in an overloaded Snowflake Warehouse. The poolsize will limit the concurrent queries which are forwarded to snowflake, but all additional Grafana queries will be stored in the query queue of the plugin. To drop new queries in the situation of overload a max. queued queries option is available. If more queries as configured are waiting a error will be returned to the panel. This will give snowflake the possibility to recover.
To implement the options in the ConfigEditor the editor code is changed from the legacy forms to the newer grafana forms.

If a Panel has multiple queries, the snowflake query will be done in multiple threads after this change.

All changes are speeding up the query execution and will reduce snowflake cloud query costs for connection creation.

This is the first pull-request. A second one implementing plugin-side caching is prepared. Please let me know if you are willing to implement such huge changes into your code base.

@devnied
Copy link
Collaborator

devnied commented Oct 7, 2024

Thanks, for this MR, I will review it asap.
For your second MR "implementing plugin-side caching" I don't think this should be a functionality of this plugin as Snowflake already perform cache query result, without extra cost.

@devnied devnied added the enhancement New feature or request label Oct 7, 2024
@MrLight
Copy link
Contributor Author

MrLight commented Oct 8, 2024

Thanks, for looking into this.
Regarding the plugin-side caching. I have a running test version of that. I do some cleaning at the moment and will publish that in my fork first. Possibly you like to take a look into it.
Out of my opinion it really helps in terms of usability, especially if you are browsing back n force or if someone sets (too) high refresh rates ...
Will ping you if its ready... (btw. thanks for merging my other pull request)

@MrLight
Copy link
Contributor Author

MrLight commented Oct 11, 2024

I have published a beta release in "my" fork. If you like you can have look there...
preview version

Copy link
Collaborator

@devnied devnied left a comment

Choose a reason for hiding this comment

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

Thank you for this great PR!
I have a few pieces of feedback based on my review.
I also need to test a few edge cases before I can approve it.

pkg/query.go Outdated Show resolved Hide resolved
pkg/query.go Outdated Show resolved Hide resolved
pkg/query.go Outdated Show resolved Hide resolved
pkg/snowflake.go Outdated Show resolved Hide resolved
pkg/snowflake.go Outdated Show resolved Hide resolved
pkg/snowflake_test.go Outdated Show resolved Hide resolved
Copy link

sonarqubecloud bot commented Nov 6, 2024

@devnied devnied added this to the 2.0.0 milestone Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants