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

SNOW-860872 connection pool #955

Merged
merged 29 commits into from
Jun 12, 2024
Merged

Conversation

sfc-gh-knozderko
Copy link
Collaborator

Description

New connection pool.

Checklist

  • Code compiles correctly
  • Code is formatted according to Coding Conventions
  • Created tests which fail without the change (if possible)
  • All tests passing (dotnet test)
  • Extended the README / documentation, if necessary
  • Provide JIRA issue id (if possible) or GitHub issue id in PR name

sfc-gh-mhofman and others added 24 commits November 9, 2023 16:31
### Description
Connection Pool v2
- introduction of connection manager handling multiple pool of sessions
- ability to switch Connection Manager between v1 and v2
- controlling pool settings for each pool independently
- default pool version: v1

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
Split integration tests for different connection pools.

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
Connection Pool v2:
- introduction of a counter of busy sessions created by Pool Manager
- pool size counts both: Idle and Busy sessions

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-937190 Wait for idle sessions available if session count exceeds
max count

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-902632 connection string driven pool config

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
Min pool size

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
Fix flaky test TestWaitUntilResourceAvailable

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
…ol (#912)

### Description
Prevent evicted connections from returning to the pool

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
Make new pool as a default one.

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-986233 Log pool status

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
- Public API to switch settings for the new pool
- Public API to get pool from the pool manger based on connection string (and
if appropriate password)
- Support for pooling (or removing) a connection when its properties
(warehouse, role, db, schema) got altered

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [ ] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name

---------

Co-authored-by: Krzysztof Nozderko <[email protected]>
### Description
SNOW-1406763 Test for password with special characters

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-1373257 Make secure password be a part of pool key

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
…cation (#947)

### Description
Disable connection pooling for external browser authentication

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
…y file (#948)

### Description
SNOW-1417631 Disable pooling for jwt token authentication with key file

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
New connection pool documentation.

### Checklist
- [ ] Code compiles correctly
- [ ] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [ ] Created tests which fail without the change (if possible)
- [ ] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name

---------

Co-authored-by: Dariusz Stempniak <[email protected]>
Co-authored-by: Krzysztof Nozderko <[email protected]>
### Description
Fix pool flaky test.

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [ ] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
Improve documentation.

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [ ] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-968914 Log closing sessions

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-968914 Break waiting on pool destroy

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
@sfc-gh-knozderko sfc-gh-knozderko requested a review from a team as a code owner May 23, 2024 09:15
Copy link

gitguardian bot commented May 23, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
9511947 Triggered ODBC Connection String 5a3c451 Snowflake.Data.Tests/UnitTests/SFOktaTest.cs View secret
9511947 Triggered ODBC Connection String 5a3c451 Snowflake.Data.Tests/UnitTests/SFOktaTest.cs View secret
🛠 Guidelines to remediate hardcoded secrets

The above secret(s) have been detected in your PR. Please take an appropriate action for each secret:

  • If it’s a true positive, remove the secret from source code, revoke it and migrate to a secure way of storing and accessing secrets (see http://go/secrets-and-code). Once that’s done, go to the incidents page linked in the “GitGuardian id” column (log in using SnowBiz Okta) and resolve the incident.
  • If it’s a false positive, go to the incidents page linked in the “GitGuardian id” column (log in using SnowBiz Okta) and ignore the incident.
  • If you didn't add this secret - and only then - you may ignore this check as it's non-blocking. If you did add the secret and you ignore this check, you'll be assigned a "Security Finding" ticket in Jira in a few days.

Note:

  • A secret is considered leaked from the moment it touches GitHub. Rewriting git history by force pushing or other means is not necessary and doesn’t change the fact that the secret has to be revoked.
  • This check has a “Skip: false positive” button. Don’t use it. It will mark all detected secrets as false positives but only in the context of this specific run - it won’t remember this action in subsequent check runs.

If you encounter any problems you can reach out to us on Slack: #gitguardian-secret-scanning-help


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Copy link

codecov bot commented May 23, 2024

Codecov Report

Attention: Patch coverage is 93.11520% with 101 lines in your changes missing coverage. Please review.

Project coverage is 85.09%. Comparing base (d158fd4) to head (3951612).

Current head 3951612 differs from pull request most recent head 53f74be

Please upload reports for the commit 53f74be to get more accurate results.

Files Patch % Lines
Snowflake.Data/Core/Session/SessionPool.cs 92.75% 21 Missing and 9 partials ⚠️
Snowflake.Data/Core/SFResultSet.cs 91.54% 15 Missing and 2 partials ⚠️
Snowflake.Data/Client/SnowflakeDbSessionPool.cs 27.27% 7 Missing and 1 partial ⚠️
...ke.Data/Core/Authenticator/KeyPairAuthenticator.cs 0.00% 7 Missing ⚠️
Snowflake.Data/Core/Session/SFSession.cs 87.50% 5 Missing and 2 partials ⚠️
Snowflake.Data/Client/SnowflakeDbConnectionPool.cs 90.62% 4 Missing and 2 partials ⚠️
...Data/Core/Session/SFSessionHttpClientProperties.cs 95.00% 4 Missing and 2 partials ⚠️
Snowflake.Data/Core/Session/WaitingQueue.cs 94.38% 3 Missing and 2 partials ⚠️
...owflake.Data/Core/Session/ConnectionPoolManager.cs 95.50% 2 Missing and 2 partials ⚠️
...flake.Data/Core/Session/SessionOrCreationTokens.cs 82.60% 2 Missing and 2 partials ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #955      +/-   ##
==========================================
+ Coverage   84.74%   85.09%   +0.34%     
==========================================
  Files          89      106      +17     
  Lines        9709    10759    +1050     
  Branches      921     1029     +108     
==========================================
+ Hits         8228     9155     +927     
- Misses       1253     1355     +102     
- Partials      228      249      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

### Description
SNOW-1337069 Fix note about special signs in connection string

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
### Description
SNOW-1452613 Update secret detector

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../blob/master/CodingConventions.md)
- [x] Created tests which fail without the change (if possible)
- [x] All tests passing (`dotnet test`)
- [x] Extended the README / documentation, if necessary
- [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
@snowflakedb snowflakedb deleted a comment from sfc-gh-mhofman Jun 11, 2024
@sfc-gh-knozderko sfc-gh-knozderko force-pushed the pool/SNOW-860872-connection-pool branch from 3951612 to 53f74be Compare June 12, 2024 10:56
@sfc-gh-knozderko sfc-gh-knozderko merged commit 1465bda into master Jun 12, 2024
33 checks passed
@sfc-gh-knozderko sfc-gh-knozderko deleted the pool/SNOW-860872-connection-pool branch June 12, 2024 11:31
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants