From fee80d010d7d2d3d30cfedba8a8093d5943fda56 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 27 Apr 2020 12:58:03 -0700 Subject: [PATCH 1/8] Fix stored procedure issues Fixes #542 --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- .../netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 02f05a7822..4f1ef3a049 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -2950,7 +2950,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // _pendingData set by e.g. 'TdsExecuteSQLBatch' // _hasOpenResult always set to true by 'WriteMarsHeader' // - if (!stateObj._attentionSent && !stateObj.HasPendingData && stateObj.HasOpenResult) + if (!stateObj.HasReceivedAttention && !stateObj.HasPendingData && stateObj.HasOpenResult) { /* Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) || diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 3ebf40d816..33e5feb579 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -3341,7 +3341,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // _pendingData set by e.g. 'TdsExecuteSQLBatch' // _hasOpenResult always set to true by 'WriteMarsHeader' // - if (!stateObj._attentionSent && !stateObj._pendingData && stateObj._hasOpenResult) + if (!stateObj._attentionReceived && !stateObj._pendingData && stateObj._hasOpenResult) { /* Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) || From 7cb481db6841fc0d4591d881355a82923abb4cc5 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 27 Apr 2020 15:50:25 -0700 Subject: [PATCH 2/8] Fix comment --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- .../netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 4f1ef3a049..f8cd77e735 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -2946,7 +2946,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio } } - // _attentionSent set by 'SendAttention' + // HasReceivedAttention set above // _pendingData set by e.g. 'TdsExecuteSQLBatch' // _hasOpenResult always set to true by 'WriteMarsHeader' // diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 33e5feb579..4f88f5eb27 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -3337,7 +3337,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio } } - // _attentionSent set by 'SendAttention' + // _attentionReceived set above // _pendingData set by e.g. 'TdsExecuteSQLBatch' // _hasOpenResult always set to true by 'WriteMarsHeader' // From 3dfc06afc6a2f736478b71a88ce55a1ebb53c8fa Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 28 Apr 2020 09:58:25 -0700 Subject: [PATCH 3/8] Add test case. --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index a8cbbdf6f4..179faa4e19 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -212,6 +212,36 @@ public static void AsyncCancelDoesNotWaitNP() AsyncCancelDoesNotWait(np_connStr).Wait(); } + [CheckConnStrSetupFact] + public static void TCPAttentionPacketTest() + { + CancelFollowedByTransaction(tcp_connStr); + } + + [CheckConnStrSetupFact] + public static void NPAttentionPacketTest() + { + CancelFollowedByTransaction(np_connStr); + } + + private static void CancelFollowedByTransaction(string constr) + { + using (SqlConnection connection = new SqlConnection(constr)) + { + connection.Open(); + using (SqlCommand cmd = connection.CreateCommand()) + { + cmd.CommandText = @"SELECT @@VERSION"; + using (var r = cmd.ExecuteReader()) + { + cmd.Cancel(); + } + } + using (var transaction = connection.BeginTransaction()) + { } + } + } + private static void MultiThreadedCancel(string constr, bool async) { using (SqlConnection con = new SqlConnection(constr)) From 3cea037c761c0c3f956a0cb1e763a854da570b3f Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 28 Apr 2020 10:26:18 -0700 Subject: [PATCH 4/8] Handle test case for named pipes --- .../tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index 179faa4e19..dd3270b8e9 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -218,7 +218,8 @@ public static void TCPAttentionPacketTest() CancelFollowedByTransaction(tcp_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + [PlatformSpecific(TestPlatforms.Windows)] public static void NPAttentionPacketTest() { CancelFollowedByTransaction(np_connStr); From 5428ec5675581ee789442bf5a9058c2c901af33e Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 28 Apr 2020 16:18:08 -0700 Subject: [PATCH 5/8] Added more test cases --- .../SQL/SqlCommand/SqlCommandCancelTest.cs | 72 ++++++++++++++++++- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index dd3270b8e9..56e6d180b3 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -213,18 +213,31 @@ public static void AsyncCancelDoesNotWaitNP() } [CheckConnStrSetupFact] - public static void TCPAttentionPacketTest() + public static void TCPAttentionPacketTestTransaction() { CancelFollowedByTransaction(tcp_connStr); } [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] - public static void NPAttentionPacketTest() + public static void NPAttentionPacketTestTransaction() { CancelFollowedByTransaction(np_connStr); } + [CheckConnStrSetupFact] + public static void TCPAttentionPacketTestAlerts() + { + CancelFollowedByAlert(tcp_connStr); + } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] + [PlatformSpecific(TestPlatforms.Windows)] + public static void NPAttentionPacketTestAlerts() + { + CancelFollowedByAlert(np_connStr); + } + private static void CancelFollowedByTransaction(string constr) { using (SqlConnection connection = new SqlConnection(constr)) @@ -238,11 +251,64 @@ private static void CancelFollowedByTransaction(string constr) cmd.Cancel(); } } - using (var transaction = connection.BeginTransaction()) + using (SqlTransaction transaction = connection.BeginTransaction()) { } } } + private static void CancelFollowedByAlert(string constr) + { + var alertName = "myAlert" + Guid.NewGuid().ToString(); + var n = new Random().Next(1, 100); + bool retry = true; + int retryAttempt = 0; + while (retry && retryAttempt < 3) + { + try + { + using (var conn = new SqlConnection(constr)) + { + conn.Open(); + using (SqlCommand cmd = conn.CreateCommand()) + { + cmd.CommandText = "SELECT @@VERSION"; + using (var reader = cmd.ExecuteReader()) + { + cmd.Cancel(); // Sends Attention + } + } + using (SqlCommand cmd = conn.CreateCommand()) + { + cmd.CommandText = $@"EXEC msdb.dbo.sp_add_alert @name=N'{alertName}', + @performance_condition = N'SQLServer:General Statistics|User Connections||>|{n}'"; + cmd.ExecuteNonQuery(); + cmd.CommandText = @"USE [msdb]"; + cmd.ExecuteNonQuery(); + cmd.CommandText = $@"/****** Object: Alert [{alertName}] Script Date: {DateTime.Now} ******/ + IF EXISTS (SELECT name FROM msdb.dbo.sysalerts WHERE name = N'{alertName}') + EXEC msdb.dbo.sp_delete_alert @name=N'{alertName}'"; + cmd.ExecuteNonQuery(); + } + } + } + catch (Exception e) + { + if (retryAttempt >= 3 || e.Message.Contains("The transaction operation cannot be performed")) + { + Assert.False(true, $"Retry Attempt: {retryAttempt} | Unexpected Exception occurred: {e.Message}"); + } + else + { + retry = true; + retryAttempt++; + Console.WriteLine($"Retry Attempt : {retryAttempt}"); + continue; + } + } + retry = false; + } + } + private static void MultiThreadedCancel(string constr, bool async) { using (SqlConnection con = new SqlConnection(constr)) From 4229fcc87183a69d7034a2a1c3ee244af47329df Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 28 Apr 2020 16:21:13 -0700 Subject: [PATCH 6/8] Skip on Azure --- .../tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index 56e6d180b3..8a00bbfa86 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -225,7 +225,7 @@ public static void NPAttentionPacketTestTransaction() CancelFollowedByTransaction(np_connStr); } - [CheckConnStrSetupFact] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] public static void TCPAttentionPacketTestAlerts() { CancelFollowedByAlert(tcp_connStr); @@ -259,6 +259,8 @@ private static void CancelFollowedByTransaction(string constr) private static void CancelFollowedByAlert(string constr) { var alertName = "myAlert" + Guid.NewGuid().ToString(); + // Since Alert conditions are randomly generated, + // we will rety on unexpected error messages to avoid collision in pipelines. var n = new Random().Next(1, 100); bool retry = true; int retryAttempt = 0; From 68782c6b5fe487fea67c069acff55c5372238b54 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 28 Apr 2020 16:22:46 -0700 Subject: [PATCH 7/8] Typo --- .../tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index 8a00bbfa86..a1e89cc167 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -260,7 +260,7 @@ private static void CancelFollowedByAlert(string constr) { var alertName = "myAlert" + Guid.NewGuid().ToString(); // Since Alert conditions are randomly generated, - // we will rety on unexpected error messages to avoid collision in pipelines. + // we will retry on unexpected error messages to avoid collision in pipelines. var n = new Random().Next(1, 100); bool retry = true; int retryAttempt = 0; From 9a2ae1bb32db935987ec114c26761a5eb7cd5f07 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Tue, 28 Apr 2020 16:31:24 -0700 Subject: [PATCH 8/8] Small Sleep time --- .../tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs index a1e89cc167..121f992d02 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs @@ -303,7 +303,8 @@ IF EXISTS (SELECT name FROM msdb.dbo.sysalerts WHERE name = N'{alertName}') { retry = true; retryAttempt++; - Console.WriteLine($"Retry Attempt : {retryAttempt}"); + Console.WriteLine($"CancelFollowedByAlert Test retry attempt : {retryAttempt}"); + Thread.Sleep(500); continue; } }