Skip to content

Conversation

@mdaigle
Copy link
Contributor

@mdaigle mdaigle commented Sep 22, 2025

Description

A Basic Availability Group (BAG), is a basic high availability configuration for standard edition, on-prem servers (not applicable to azure). It involves two servers, one a primary replica and the other a secondary replica (a.k.a. failover partner / mirroring partner). When the primary is unavailable, clients will connect to the secondary. When connecting to the primary in a BAG, the server specifies the domain name of the secondary to the client via a Real Time Log Shipping ENVCHANGE token. This allows the BAG's secondary configuration to be updated dynamically without a code change. We call this the server-provided failover partner. Clients can also specify a failover partner directly in the connection string. The client-provided failover partner is used only if the initial connection to the primary replica fails (no server-side failover partner available). Otherwise, the server-provided failover partner supersedes the client-provided failover partner.

When configuring a BAG, replicas are added to the group by specifying their NetBIOS or network name (CREATE AVAILABILITY GROUP (Transact-SQL) - SQL Server | Microsoft Learn), NOT their protocol, fully qualified domain name, and port. This network name is the value returned to the client as the server-provided failover partner. This causes an issue when using TCP for either replica because the server-provided failover partner will not contain the protocol and FQDN.

The goal of this PR is to provide support for TCP in BAGs while respecting the following constraints:

  • We must continue to prefer server-provided failover partners by default. This behavior is useful and changing it would not be backwards compatible.
  • Low cost. Avoid server-side or TSQL changes.

We introduce a client-side app context switch that, when enabled, ignores the server-provided failover partner and only uses the failover partner provided in the connection string.

Example usage

Connecting to a BAG via TCP:

AppContext.SetSwitch("Switch.Microsoft.Data.SqlClient.IgnoreServerProvidedFailoverPartner", true);

SqlConnection connection = new SqlConnection("Data Source=tcp:localhost,1234;Failover Partner=tcp:localhost,5678;...");
connection.Open();

Issues

#3400

Testing

Testing capabilities are currently limited in this area.
#3488 will add basic integration level test coverage for this area.

@mdaigle mdaigle requested a review from a team as a code owner September 22, 2025 22:47
Copilot AI review requested due to automatic review settings September 22, 2025 22:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces an app context switch to ignore server-provided failover partners in Basic Availability Groups (BAGs), addressing issues with custom ports when the server provides only the NetBIOS name without port information.

  • Adds IgnoreServerProvidedFailoverPartner app context switch to allow client-side control of failover behavior
  • Refactors failover partner property from private field to public property for better encapsulation
  • Updates failover logic to conditionally ignore server-provided partners based on the new switch

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
LocalAppContextSwitches.cs Adds new app context switch configuration and property
SqlInternalConnectionTds.cs (netfx) Implements conditional failover partner logic and refactors property access
SqlInternalConnectionTds.cs (netcore) Mirror implementation for .NET Core with same conditional logic
SqlCommand.cs (netfx) Updates property name reference for consistency
SqlCommand.cs (netcore) Updates property name reference for consistency

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Sep 23, 2025

Can you add an example here on how should a user specify secondary failover replica info with this change and enable app context switch alongside?

@edwardneal
Copy link
Contributor

This might also be the root cause of #2610.

Besides this, another issue (#2365) has suggested a similar idea, but controlled using a connection string option.

@mdaigle
Copy link
Contributor Author

mdaigle commented Sep 23, 2025

This might also be the root cause of #2610.

Besides this, another issue (#2365) has suggested a similar idea, but controlled using a connection string option.

Yes, looks like this is the same issue. The network name used to configure a BAG doesn't square with most people's setups these days.

@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.82%. Comparing base (d244be2) to head (5e0b12e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3625       +/-   ##
===========================================
+ Coverage   66.43%   90.82%   +24.38%     
===========================================
  Files         271        6      -265     
  Lines       60277      316    -59961     
===========================================
- Hits        40048      287    -39761     
+ Misses      20229       29    -20200     
Flag Coverage Δ
addons 90.82% <ø> (?)
netcore ?
netfx ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

paulmedynski
paulmedynski previously approved these changes Sep 23, 2025
@paulmedynski paulmedynski self-assigned this Sep 23, 2025
cheenamalhotra
cheenamalhotra previously approved these changes Sep 23, 2025
@cheenamalhotra cheenamalhotra added this to the 7.0-preview2 milestone Sep 23, 2025
@mdaigle mdaigle dismissed stale reviews from cheenamalhotra and paulmedynski via 5e0b12e September 26, 2025 16:37
@mdaigle mdaigle merged commit fa7132e into main Sep 29, 2025
252 checks passed
@mdaigle mdaigle deleted the dev/mdaigle/38489-retain-failover-port branch September 29, 2025 16:11
mdaigle added a commit that referenced this pull request Oct 20, 2025
* Add IgnoreServerProvidedFailoverPartner app context switch.

* Add behavior skip to netfx.

* Consolidate to single property for failover partner value.

* Rework checks to preserve server provided value, but ignore it.

* Fix import.

* Skip server failover partner override in �LoginNoFailover method. Add simulated server test coverage.
mdaigle added a commit that referenced this pull request Oct 20, 2025
* Add IgnoreServerProvidedFailoverPartner app context switch.

* Add behavior skip to netfx.

* Consolidate to single property for failover partner value.

* Rework checks to preserve server provided value, but ignore it.

* Fix import.

* Skip server failover partner override in �LoginNoFailover method. Add simulated server test coverage.
mdaigle added a commit that referenced this pull request Oct 28, 2025
…ner. (#3704)

* Fix #3400, ignore server-provided failover partner (#3625)

* Add IgnoreServerProvidedFailoverPartner app context switch.

* Add behavior skip to netfx.

* Consolidate to single property for failover partner value.

* Rework checks to preserve server provided value, but ignore it.

* Fix import.

* Skip server failover partner override in �LoginNoFailover method. Add simulated server test coverage.

* Fix compilation issues.

* Clean up duplicate line. Adjust app context switch logic to match existing.

* Review changes.

* Add test case. Doesn't compile.

* Adjust usage of test servers and app context.

* Add failover and prelogin count capabilities to test server. Update test case.

* Set backing field using reflection.

* Try clearing pool to make test more reliable.

* Use string.Empty
mdaigle added a commit that referenced this pull request Oct 29, 2025
* Add IgnoreServerProvidedFailoverPartner app context switch.

* Add behavior skip to netfx.

* Consolidate to single property for failover partner value.

* Rework checks to preserve server provided value, but ignore it.

* Fix import.

* Skip server failover partner override in �LoginNoFailover method. Add simulated server test coverage.
mdaigle added a commit that referenced this pull request Oct 31, 2025
…ner. (#3702)

* Fix #3400, ignore server-provided failover partner (#3625)

* Add IgnoreServerProvidedFailoverPartner app context switch.

* Add behavior skip to netfx.

* Consolidate to single property for failover partner value.

* Rework checks to preserve server provided value, but ignore it.

* Fix import.

* Skip server failover partner override in �LoginNoFailover method. Add simulated server test coverage.

* Adjust category level so test runs in ci.

* Review changes.

* Clear pool to make test more reliable.

* Serialize unit tests that touch app context switches

* Are connections getting carried over in the pool somehow?

* Skip test in pipeline.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

6 participants