-
Notifications
You must be signed in to change notification settings - Fork 315
Enhanced routing envchange token support #3646
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
Open
mdaigle
wants to merge
20
commits into
feat/enhanced-routing
Choose a base branch
from
dev/mdaigle/enhanced-routing-envchange-token
base: feat/enhanced-routing
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
4c0acd3
Add feature extension, request by default, process ack.
mdaigle 2136370
Add support to netfx. Add simulated server tests.
mdaigle 40af1cf
review changes
mdaigle 924218a
Add enhanced routing to TdsEnums.
mdaigle fabad0a
Remove unused envchange enum.
mdaigle 702ecf1
Add database name to RoutingInfo dto.
mdaigle 91bade2
Add new user facing exception message for invalid enhanced routing info.
mdaigle eb8d271
Add enhanced routing token processing from stream.
mdaigle 5aff345
Add logic to validate and store enhanced routing info on the internal…
mdaigle b850421
Convert some fields to auto properties to reduce clutter.
mdaigle 2fc8175
Clean up argument casting in RoutingTdsServer.
mdaigle 7dcafb8
Add database name to routing server arguments.
mdaigle 1d9b7dd
Add enhanced routing envchange token value for use by simulated serve…
mdaigle 2d422fb
Add enhanced routing envchange type to enum
mdaigle 2456f28
Return enhanced routing token when server is configured with a routin…
mdaigle 0b6289d
Enable default compile items for TDS proj.
mdaigle 8cd2270
Clean up envchange token initialization.
mdaigle b1aee5e
Fix ResolvedDatabaseName update.
mdaigle 05ca818
Fix enhanced routing inflation and deflation for simulated servers.
mdaigle 3486a2f
Add simulated server tests.
mdaigle File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -208,11 +208,14 @@ internal bool IsDNSCachingBeforeRedirectSupported | |
// Json Support Flag | ||
internal bool IsJsonSupportEnabled = false; | ||
|
||
// Vector Support Flag | ||
internal bool IsVectorSupportEnabled = false; | ||
|
||
// User Agent Flag | ||
internal bool IsUserAgentEnabled = true; | ||
|
||
// Vector Support Flag | ||
internal bool IsVectorSupportEnabled = false; | ||
// Enhanced Routing Support Flag | ||
internal bool IsEnhancedRoutingSupportEnabled = false; | ||
|
||
// TCE flags | ||
internal byte _tceVersionSupported; | ||
|
@@ -316,7 +319,6 @@ internal SessionData CurrentSessionData | |
private string _originalDatabase; | ||
private string _originalLanguage; | ||
private string _currentLanguage; | ||
private int _currentPacketSize; | ||
private int _asyncCommandCount; // number of async Begins minus number of async Ends. | ||
|
||
// FOR SSE | ||
|
@@ -439,9 +441,6 @@ internal SqlConnectionTimeoutErrorInternal TimeoutErrorInternal | |
// OTHER STATE VARIABLES AND REFERENCES | ||
internal Guid _clientConnectionId = Guid.Empty; | ||
|
||
// Routing information (ROR) | ||
private Guid _originalClientConnectionId = Guid.Empty; | ||
private string _routingDestination = null; | ||
private readonly TimeoutTimer _timeout; | ||
|
||
// although the new password is generally not used it must be passed to the ctor | ||
|
@@ -605,21 +604,9 @@ internal Guid ClientConnectionId | |
} | ||
} | ||
|
||
internal Guid OriginalClientConnectionId | ||
{ | ||
get | ||
{ | ||
return _originalClientConnectionId; | ||
} | ||
} | ||
internal Guid OriginalClientConnectionId { get; private set; } = Guid.Empty; | ||
|
||
internal string RoutingDestination | ||
{ | ||
get | ||
{ | ||
return _routingDestination; | ||
} | ||
} | ||
internal string RoutingDestination { get; private set; } | ||
|
||
internal RoutingInfo RoutingInfo { get; private set; } = null; | ||
|
||
|
@@ -687,13 +674,7 @@ internal override bool Is2008OrNewer | |
} | ||
} | ||
|
||
internal int PacketSize | ||
{ | ||
get | ||
{ | ||
return _currentPacketSize; | ||
} | ||
} | ||
internal int PacketSize { get; private set; } | ||
|
||
internal TdsParser Parser | ||
{ | ||
|
@@ -1295,7 +1276,7 @@ private void Login(ServerInfo server, TimeoutTimer timeout, string newPassword, | |
// gather all the settings the user set in the connection string or | ||
// properties and do the login | ||
CurrentDatabase = server.ResolvedDatabaseName; | ||
_currentPacketSize = ConnectionOptions.PacketSize; | ||
PacketSize = ConnectionOptions.PacketSize; | ||
_currentLanguage = ConnectionOptions.CurrentLanguage; | ||
|
||
int timeoutInSeconds = 0; | ||
|
@@ -1342,7 +1323,7 @@ private void Login(ServerInfo server, TimeoutTimer timeout, string newPassword, | |
login.useReplication = ConnectionOptions.Replication; | ||
login.useSSPI = ConnectionOptions.IntegratedSecurity // Treat AD Integrated like Windows integrated when against a non-FedAuth endpoint | ||
|| (ConnectionOptions.Authentication == SqlAuthenticationMethod.ActiveDirectoryIntegrated && !_fedAuthRequired); | ||
login.packetSize = _currentPacketSize; | ||
login.packetSize = PacketSize; | ||
login.newPassword = newPassword; | ||
login.readOnlyIntent = ConnectionOptions.ApplicationIntent == ApplicationIntent.ReadOnly; | ||
login.credential = _credential; | ||
|
@@ -1410,6 +1391,7 @@ private void Login(ServerInfo server, TimeoutTimer timeout, string newPassword, | |
requestedFeatures |= TdsEnums.FeatureExtension.SQLDNSCaching; | ||
requestedFeatures |= TdsEnums.FeatureExtension.JsonSupport; | ||
requestedFeatures |= TdsEnums.FeatureExtension.VectorSupport; | ||
requestedFeatures |= TdsEnums.FeatureExtension.EnhancedRoutingSupport; | ||
|
||
#if DEBUG | ||
requestedFeatures |= TdsEnums.FeatureExtension.UserAgent; | ||
|
@@ -1649,11 +1631,11 @@ private void LoginNoFailover(ServerInfo serverInfo, | |
|
||
serverInfo = new ServerInfo(ConnectionOptions, RoutingInfo, serverInfo.ResolvedServerName, serverInfo.ServerSPN); | ||
_timeoutErrorInternal.SetInternalSourceType(SqlConnectionInternalSourceType.RoutingDestination); | ||
_originalClientConnectionId = _clientConnectionId; | ||
_routingDestination = serverInfo.UserServerName; | ||
OriginalClientConnectionId = _clientConnectionId; | ||
RoutingDestination = serverInfo.UserServerName; | ||
|
||
// restore properties that could be changed by the environment tokens | ||
_currentPacketSize = ConnectionOptions.PacketSize; | ||
PacketSize = ConnectionOptions.PacketSize; | ||
_currentLanguage = _originalLanguage = ConnectionOptions.CurrentLanguage; | ||
CurrentDatabase = _originalDatabase = ConnectionOptions.InitialCatalog; | ||
ServerProvidedFailoverPartner = null; | ||
|
@@ -1916,11 +1898,11 @@ TimeoutTimer timeout | |
|
||
currentServerInfo = new ServerInfo(ConnectionOptions, RoutingInfo, currentServerInfo.ResolvedServerName, currentServerInfo.ServerSPN); | ||
_timeoutErrorInternal.SetInternalSourceType(SqlConnectionInternalSourceType.RoutingDestination); | ||
_originalClientConnectionId = _clientConnectionId; | ||
_routingDestination = currentServerInfo.UserServerName; | ||
OriginalClientConnectionId = _clientConnectionId; | ||
RoutingDestination = currentServerInfo.UserServerName; | ||
|
||
// restore properties that could be changed by the environment tokens | ||
_currentPacketSize = connectionOptions.PacketSize; | ||
PacketSize = connectionOptions.PacketSize; | ||
_currentLanguage = _originalLanguage = ConnectionOptions.CurrentLanguage; | ||
CurrentDatabase = _originalDatabase = connectionOptions.InitialCatalog; | ||
ServerProvidedFailoverPartner = null; | ||
|
@@ -2204,7 +2186,7 @@ internal void OnEnvChange(SqlEnvChange rec) | |
break; | ||
|
||
case TdsEnums.ENV_PACKETSIZE: | ||
_currentPacketSize = int.Parse(rec._newValue, CultureInfo.InvariantCulture); | ||
PacketSize = int.Parse(rec._newValue, CultureInfo.InvariantCulture); | ||
break; | ||
|
||
case TdsEnums.ENV_COLLATION: | ||
|
@@ -2277,6 +2259,21 @@ internal void OnEnvChange(SqlEnvChange rec) | |
RoutingInfo = rec._newRoutingInfo; | ||
break; | ||
|
||
case TdsEnums.ENV_ENHANCEDROUTING: | ||
SqlClientEventSource.Log.TryAdvancedTraceEvent("<sc.SqlInternalConnectionTds.OnEnvChange|ADV> {0}, Received enhanced routing info", ObjectID); | ||
|
||
if (string.IsNullOrEmpty(rec._newRoutingInfo.ServerName) || | ||
string.IsNullOrEmpty(rec._newRoutingInfo.DatabaseName) || | ||
rec._newRoutingInfo.Protocol != 0 || | ||
rec._newRoutingInfo.Port == 0) | ||
{ | ||
throw SQL.ROR_InvalidEnhancedRoutingInfo(this); | ||
} | ||
|
||
RoutingInfo = rec._newRoutingInfo; | ||
|
||
break; | ||
|
||
default: | ||
Debug.Fail("Missed token in EnvChange!"); | ||
break; | ||
|
@@ -3014,6 +3011,20 @@ internal void OnFeatureExtAck(int featureId, byte[] data) | |
break; | ||
} | ||
|
||
case TdsEnums.FEATUREEXT_ENHANCEDROUTINGSUPPORT: | ||
{ | ||
SqlClientEventSource.Log.TryAdvancedTraceEvent("<sc.SqlInternalConnectionTds.OnFeatureExtAck|ADV> {0}, Received feature extension acknowledgement for ENHANCEDROUTINGSUPPORT", ObjectID); | ||
if (data.Length != 1) | ||
{ | ||
SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.OnFeatureExtAck|ERR> {0}, Unknown token for ENHANCEDROUTINGSUPPORT", ObjectID); | ||
throw SQL.ParsingError(ParsingErrorState.CorruptedTdsStream); | ||
} | ||
|
||
// A value of 1 indicates that the server supports the feature. | ||
IsEnhancedRoutingSupportEnabled = data[0] == 1; | ||
break; | ||
} | ||
|
||
default: | ||
{ | ||
// Unknown feature ack | ||
|
@@ -3134,10 +3145,19 @@ internal ServerInfo(SqlConnectionString userOptions, RoutingInfo routing, string | |
{ | ||
UserServerName = string.Format(CultureInfo.InvariantCulture, "{0},{1}", routing.ServerName, routing.Port); | ||
} | ||
|
||
if (routing == null || routing.DatabaseName == null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this statement be easier to understand if it were flipped?
|
||
{ | ||
ResolvedDatabaseName = userOptions.InitialCatalog; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can has ternary ? 👉👈🥺 |
||
} | ||
else | ||
{ | ||
ResolvedDatabaseName = routing.DatabaseName; | ||
} | ||
|
||
PreRoutingServerName = preRoutingServerName; | ||
UserProtocol = TdsEnums.TCP; | ||
SetDerivedNames(UserProtocol, UserServerName); | ||
ResolvedDatabaseName = userOptions.InitialCatalog; | ||
ServerSPN = serverSPN; | ||
} | ||
|
||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the tracing format we're trying to use going forward is the
xyz | abc | whatever
format. But if the rest of the strings in this file are using the<abc...> asdfasdfads
format, then I won't complain.