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

[BUG] : SQL connection pool issue when using the MSI token for SQL generated with Azure.Identity #28572

Closed
RanjanMishra92 opened this issue May 6, 2022 · 10 comments
Assignees
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@RanjanMishra92
Copy link

Library name and version

Azure.Identity 1.5.0

Describe the bug

We migrated our solution from ADAL to MSAL , and in our code we were using Microsoft.Azure.Services.AppAuthentication which was used to get the MSI token and that token was used by SQL (entity framework core) to authenticate. As this Microsoft.Azure.Services.AppAuthentication package has a dependency on ADAL we have replaced it with Azure.Identity 1.5.0.
So when we use the MSI token generated for SQL with Azure.Identity , SQL connection pooling seems to be broken , as we started getting below error related to the SQL connection pool.
Error :
Resource ID : 2. The session limit for the elastic pool is 30000 and has been reached. See 'http://go.microsoft.com/fwlink/?LinkId=267637' for assistance.
Changed database context to ''.
Changed language setting to us_english.

when we keep all settings same and switch back to old code which was using MSI tokens generated from : Microsoft.Azure.Services.AppAuthentication we do not see this error on same load test execution.

Expected behavior

The expected behavior is that it should not break the SQL connection pooling with MSI token generated with Azure.Identity.

Actual behavior

We are getting SQL connection pooling error with MSI token generated with Azure.Identity,

Error :
Resource ID : 2. The session limit for the elastic pool is 30000 and has been reached. See 'http://go.microsoft.com/fwlink/?LinkId=267637' for assistance.
Changed database context to ''.
Changed language setting to us_english.

Reproduction Steps

  1. Generate the MSI token for SQL resource
    var tokenCredential = new Azure.Identity.DefaultAzureCredential();
    var managedIdentityTokenForSql = tokenCredential.GetToken(
    new Azure.Core.TokenRequestContext(
    new[] { "https://database.windows.net/.default" }, tenantId: "your_tenantId")).Token;

  2. Now use this token and pass to the entity framework core :
    var connection = Database.GetDbConnection() as SqlConnection;
    if (connection != null)
    {
    connection.AccessToken = managedIdentityTokenForSql;
    }

  3. perform load test on this where more number of transactions are executed against SQL DB.

Environment

  1. The Server API Layer which interacts with DB through Entity framework code is deployed to Azure Web App service.
  2. Managed identity is enabled on the that Azure App service.
  3. The web app is built using .NET core 3.1 version.
  4. EF core Version=3.1.7.0 .
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels May 6, 2022
@azure-sdk azure-sdk added Azure.Identity Client This issue points to a problem in the data-plane of the library. needs-team-triage Workflow: This issue needs the team to triage. labels May 6, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label May 6, 2022
@jsquire jsquire added needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team and removed needs-team-triage Workflow: This issue needs the team to triage. labels May 6, 2022
@jsquire
Copy link
Member

jsquire commented May 6, 2022

//cc: @christothes

@jsquire
Copy link
Member

jsquire commented May 6, 2022

Thank you for your feedback. Tagging and routing to the team member best able to assist.

@ShivangiReja
Copy link
Member

If you want to use DefaultAzureCredential, you can pass Authentication=Active Directory Default in your connection string. You can refer the docs here to see how to pass it.

@RanjanMishra92
Copy link
Author

@ShivangiReja,
In our current project we are using Microsoft.EntityFrameworkCore.SqlServer version 3.1.7, which in turn uses Microsoft.Data.SqlClient version 1.1.3, when we do the suggested changes we are getting below error :
"Invalid value for key 'authentication'" .
Can you please confirm if there is any version specific version of Microsoft.Data.SqlClient that we need to use for "Authentication=Active Directory Default" feature to work.

@ShivangiReja
Copy link
Member

@cheenamalhotra
Copy link
Member

@DavoudEshtehari
cc @David-Engel

@cheenamalhotra
Copy link
Member

Adding context, EntityFrameworkCore.SqlServer v3.1.7 depends on Microsoft.Data.SqlClient v1.1.3.
This issue regarding connection pooling with access token was fixed in v2.x of SqlClient (ref: dotnet/SqlClient#443)

@David-Engel
Copy link

To explain a little bit how connection pooling works in SqlClient with regard to access tokens, each different access token will have a different pool in SqlClient. So if you are stress testing and generating a new access token for 30,000 connections, you will get 30,000 connections against the database and 30,000 different connection pools.

From your repro steps, I can't tell if you are performing steps 1 and 2 for each connection, or if you are using the same access token (just step 2) for all connections. Based on the results, it sounds like you are doing the former. The latter would result in a connection pool with only the number of physical database connections up to MaxPoolSize.

Either way, upgrading to a newer version of Microsoft.Data.SqlClient that supports Authentication = "Active Directory Managed Identity" or "Active Directory Default" will work much better as SqlClient will handle token acquisition and renewal for you (you won't need step 1 at all). You will just need the Authentication option in your connection string.

@ShivangiReja ShivangiReja added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Sep 2, 2022
@ghost
Copy link

ghost commented Sep 2, 2022

Hi @RanjanMishra92. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

@ghost ghost removed the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Sep 2, 2022
@ghost
Copy link

ghost commented Sep 9, 2022

Hi @RanjanMishra92, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.

@ghost ghost closed this as completed Sep 9, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Identity Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

No branches or pull requests

7 participants