diff --git a/docs/change_notes.md b/docs/change_notes.md index 6217c2e6..6d1fc988 100644 --- a/docs/change_notes.md +++ b/docs/change_notes.md @@ -5,6 +5,7 @@ ### Version 13.0.1 - *FIXED:* Updated distribution algorithm for `--concurrencytype` of `Server` and `MaxPerServer` when number of Batch nodes is very close to the number of SQL Server targets. Was yielding less than the number of nodes. +- *FIXED:* Updated Service Bus message retrieval to better manage when messages not matching the job name are in large quantity ### Version 13.0.0 diff --git a/src/SqlBuildManager.Console.ExternalTest/CliTests.cs b/src/SqlBuildManager.Console.ExternalTest/CliTests.cs index 7f56c6fb..8129066a 100644 --- a/src/SqlBuildManager.Console.ExternalTest/CliTests.cs +++ b/src/SqlBuildManager.Console.ExternalTest/CliTests.cs @@ -63,6 +63,7 @@ private void StdErrorReader() private string linuxSettingsFilePath; private string settingsFileKeyPath; + private string logFile; [TestInitialize] public void ConfigureProcessInfo() { @@ -77,6 +78,12 @@ public void ConfigureProcessInfo() bool ds; (ds, this.cmdLine)= Cryptography.DecryptSensitiveFields(cmdLine); this.overrideFileContents = File.ReadAllLines(this.overrideFilePath).ToList(); + + } + [TestCleanup] + public void CleanUp() + { + } #region Helpers @@ -138,7 +145,7 @@ private string CreateDacpac(CommandLineArgs cmdLine, string server, string datab } private static string GetUniqueBatchJobName() { - string name = DateTime.Now.ToString("yyyy-MM-dd-HH-mm-ss") + "-" + Guid.NewGuid().ToString().Replace("-", "").Substring(0, 6); + string name = DateTime.Now.ToString("yyyy-MM-dd-HH-mm-ss") + "-" + Guid.NewGuid().ToString().ToLower().Replace("-", "").Substring(0, 6); return name; } #endregion @@ -669,8 +676,9 @@ public void BatchQuery_UpdateFail(string batchMethod, string settingsFile) [DataRow("run", "TestConfig/settingsfile-windows-queue.json")] [DataRow("run", "TestConfig/settingsfile-linux-queue.json")] [DataTestMethod] - public void Batch_Queue_SBMSource_Success(string batchMethod, string settingsFile) + public void Batch_Queue_SBMSource_ByServer_Success(string batchMethod, string settingsFile) { + settingsFile = Path.GetFullPath(settingsFile); string sbmFileName = Path.GetFullPath("SimpleSelect.sbm"); if (!File.Exists(sbmFileName)) { @@ -708,16 +716,205 @@ public void Batch_Queue_SBMSource_Success(string batchMethod, string settingsFil result = val.Result; Assert.AreEqual(0, result, StandardExecutionErrorMessage()); - //TODO: Get output to examine - //Assert.IsTrue(this.output.Contains("Completed Successfully"), "This test was should have worked"); - //if (batchMethod == "run") - //{ - // Assert.IsTrue(this.output.Contains($"Batch complete"), $"Should indicate that this was run as a batch job"); - //} - //if (batchMethod == "runthreaded") - //{ - // Assert.IsTrue(this.output.Contains($"Total number of targets: {this.overrideFileContents.Count()}"), $"Should have run against a {this.overrideFileContents.Count()} databases"); - //} + + } + + [DataRow("runthreaded", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-linux-queue.json")] + [DataTestMethod] + public void Batch_Queue_SBMSource_MaxPerserver_Success(string batchMethod, string settingsFile) + { + settingsFile = Path.GetFullPath(settingsFile); + string sbmFileName = Path.GetFullPath("SimpleSelect.sbm"); + if (!File.Exists(sbmFileName)) + { + File.WriteAllBytes(sbmFileName, Properties.Resources.SimpleSelect); + } + string jobName = GetUniqueBatchJobName(); + + var args = new string[]{ + "batch", "enqueue", + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override" , this.overrideFilePath, + "--concurrencytype", ConcurrencyType.MaxPerServer.ToString(), + "--jobname", jobName}; + + RootCommand rootCommand = CommandLineConfig.SetUp(); + Task val = rootCommand.InvokeAsync(args); + val.Wait(); + var result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + + args = new string[]{ + "batch", batchMethod, + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override", this.overrideFilePath, + "--packagename", sbmFileName, + "--concurrencytype", ConcurrencyType.Server.ToString(), + "--concurrency", "5", + "--jobname", jobName }; + + val = rootCommand.InvokeAsync(args); + val.Wait(); + result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + + } + + [DataRow("runthreaded", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-linux-queue.json")] + [DataTestMethod] + public void Batch_Queue_SBMSource_Count_Success(string batchMethod, string settingsFile) + { + settingsFile = Path.GetFullPath(settingsFile); + string sbmFileName = Path.GetFullPath("SimpleSelect.sbm"); + if (!File.Exists(sbmFileName)) + { + File.WriteAllBytes(sbmFileName, Properties.Resources.SimpleSelect); + } + string jobName = GetUniqueBatchJobName(); + + var args = new string[]{ + "batch", "enqueue", + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override" , this.overrideFilePath, + "--concurrencytype", ConcurrencyType.Count.ToString(), + "--jobname", jobName}; + + RootCommand rootCommand = CommandLineConfig.SetUp(); + Task val = rootCommand.InvokeAsync(args); + val.Wait(); + var result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + + args = new string[]{ + "batch", batchMethod, + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override", this.overrideFilePath, + "--packagename", sbmFileName, + "--concurrencytype", ConcurrencyType.Server.ToString(), + "--concurrency", "5", + "--jobname", jobName }; + + val = rootCommand.InvokeAsync(args); + val.Wait(); + result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + + } + + [DataRow("runthreaded", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-linux-queue.json")] + [DataTestMethod] + public void Batch_Queue_PlatinumDbSource_Success(string batchMethod, string settingsFile) + { + settingsFile = Path.GetFullPath(settingsFile); + int removeCount = 1; + string server, database; + string firstOverride = this.overrideFileContents.First(); + (server, database) = DatabaseHelper.ExtractServerAndDbFromLine(firstOverride); + + string minusFirst = Path.GetFullPath("TestConfig/minusFirst.cfg"); + File.WriteAllLines(minusFirst, DatabaseHelper.ModifyTargetList(this.overrideFileContents, removeCount)); + + string jobName = GetUniqueBatchJobName(); + + var args = new string[]{ + "batch", "enqueue", + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override" , minusFirst, + "--concurrencytype", ConcurrencyType.Count.ToString(), + "--jobname", jobName}; + + RootCommand rootCommand = CommandLineConfig.SetUp(); + Task val = rootCommand.InvokeAsync(args); + val.Wait(); + var result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + + args = new string[]{ + "batch", batchMethod, + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override", minusFirst, + "--platinumdbsource", database, + "--platinumserversource", server, + "--concurrencytype", ConcurrencyType.Server.ToString(), + "--concurrency", "5", + "--jobname", jobName }; + + val = rootCommand.InvokeAsync(args); + val.Wait(); + result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + } + + [DataRow("runthreaded", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-windows-queue.json")] + [DataRow("run", "TestConfig/settingsfile-linux-queue.json")] + [DataTestMethod] + public void Batch_Queue_DacpacSource_Success(string batchMethod, string settingsFile) + { + settingsFile = Path.GetFullPath(settingsFile); + int removeCount = 1; + string server, database; + string firstOverride = this.overrideFileContents.First(); + (server, database) = DatabaseHelper.ExtractServerAndDbFromLine(firstOverride); + + string minusFirst = Path.GetFullPath("TestConfig/minusFirst.cfg"); + File.WriteAllLines(minusFirst, DatabaseHelper.ModifyTargetList(this.overrideFileContents, removeCount)); + + DatabaseHelper.CreateRandomTable(this.cmdLine, firstOverride); + + string dacpacName = CreateDacpac(this.cmdLine, server, database); + Assert.IsNotNull(dacpacName, $"There was a problem creating the dacpac for this test\r\n{StandardExecutionErrorMessage()}"); + + string jobName = GetUniqueBatchJobName(); + + var args = new string[]{ + "batch", "enqueue", + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override" , minusFirst, + "--concurrencytype", ConcurrencyType.Count.ToString(), + "--jobname", jobName}; + + RootCommand rootCommand = CommandLineConfig.SetUp(); + Task val = rootCommand.InvokeAsync(args); + val.Wait(); + var result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); + + args = new string[]{ + "batch", batchMethod, + "--settingsfile", settingsFile, + "--settingsfilekey", this.settingsFileKeyPath, + "--override", minusFirst, + "--platinumdacpac", dacpacName, + "--concurrencytype", ConcurrencyType.Server.ToString(), + "--concurrency", "5", + "--jobname", jobName }; + + val = rootCommand.InvokeAsync(args); + val.Wait(); + result = val.Result; + + Assert.AreEqual(0, result, StandardExecutionErrorMessage()); } diff --git a/src/SqlBuildManager.Console.ExternalTest/SqlBuildManager.Console.ExternalTest.csproj b/src/SqlBuildManager.Console.ExternalTest/SqlBuildManager.Console.ExternalTest.csproj index d18e019d..8026e6bf 100644 --- a/src/SqlBuildManager.Console.ExternalTest/SqlBuildManager.Console.ExternalTest.csproj +++ b/src/SqlBuildManager.Console.ExternalTest/SqlBuildManager.Console.ExternalTest.csproj @@ -24,6 +24,14 @@ Always + + + + + + + + True diff --git a/src/SqlBuildManager.Console.UnitTest/ConcurrencyTest.cs b/src/SqlBuildManager.Console.UnitTest/ConcurrencyTest.cs index c41746d0..6310ab0f 100644 --- a/src/SqlBuildManager.Console.UnitTest/ConcurrencyTest.cs +++ b/src/SqlBuildManager.Console.UnitTest/ConcurrencyTest.cs @@ -40,7 +40,7 @@ internal static (string, MultiDbData) GetMultiDbData() } internal static (string, MultiDbData) CreateDefinedMultiDbData(int serverCount, int[] dbCount) { - if(serverCount != dbCount.Length) + if (serverCount != dbCount.Length) { return ("", null); } @@ -78,11 +78,11 @@ internal static (string, MultiDbData) CreateRandomizedMultiDbData(int serverCoun Random rnd = new Random(); StringBuilder sb = new StringBuilder(); matrix = new int[serverCount]; - for (int s=0;s< serverCount;s++) + for (int s = 0; s < serverCount; s++) { var dbCount = rnd.Next(minDbCount, maxDbCount + 1); matrix[s] = dbCount; - for(int d = 0;d + [DataRow( 3, 8, new int[] { 92, 225, 126, 135, 266, 186, 280, 115 })] [DataRow(26, 27, new int[] { 554, 436, 194, 441, 382, 440, 337, 242, 85, 449, 513, 426, 475, 151, 507, 460, 138, 425, 529, 120, 262, 117, 123, 391, 344, 260, 119 })] //Actual:<23> [DataRow(32, 38, new int[] { 218, 532, 396, 63, 227, 207, 185, 106, 556, 453, 528, 476, 512, 395, 73, 487, 121, 75, 450, 560, 456, 199, 488, 413, 311, 439, 132, 405, 448, 238, 266, 101, 368, 84, 133, 171, 31, 276 })] //Actual:<30> [DataRow(48, 52, new int[] { 155, 365, 406, 341, 92, 116, 294, 268, 495, 239, 260, 250, 214, 101, 190, 212, 319, 277, 137, 316, 199, 428, 198, 353, 166, 408, 239, 45, 71, 458, 231, 140, 129, 117, 451, 211, 168, 320, 378, 448, 337, 161, 149, 99, 178, 198, 43, 151, 131, 211, 407, 361 })] // Actual:<46>. diff --git a/src/SqlBuildManager.Console/Batch/BatchExecution.cs b/src/SqlBuildManager.Console/Batch/BatchExecution.cs index aab9afd0..654b30d5 100644 --- a/src/SqlBuildManager.Console/Batch/BatchExecution.cs +++ b/src/SqlBuildManager.Console/Batch/BatchExecution.cs @@ -529,15 +529,14 @@ private int ValidateBatchArgs(CommandLineArgs cmdLine, BatchType batchType) string jobToken = DateTime.Now.ToString("yyyy-MM-dd-HHmm-ss-fff"); if (!string.IsNullOrWhiteSpace(cmdLine.BatchArgs.BatchJobName)) { - cmdLine.BatchArgs.BatchJobName = Regex.Replace(cmdLine.BatchArgs.BatchJobName, "[^a-zA-Z0-9]", ""); - cmdLine.BatchArgs.BatchJobName = cmdLine.BatchArgs.BatchJobName.ToLower(); - if (cmdLine.BatchArgs.BatchJobName.Length > 47) + if (cmdLine.BatchArgs.BatchJobName.Length < 3 || cmdLine.BatchArgs.BatchJobName.Length > 41 || !Regex.IsMatch(cmdLine.BatchArgs.BatchJobName, @"^[a-z0-9]+(-[a-z0-9]+)*$")) { - cmdLine.BatchArgs.BatchJobName = cmdLine.BatchArgs.BatchJobName.Substring(0, 47); + throw new ArgumentException("The batch job name must be lower case, between 3 and 41 characters in length, and the only special character allowed are dashes '-'"); } - jobId = cmdLine.BatchArgs.BatchJobName + "-" + jobToken; + + jobId = cmdLine.BatchArgs.BatchJobName; poolId = PoolName; - storageContainerName = cmdLine.BatchArgs.BatchJobName; + storageContainerName = cmdLine.BatchArgs.BatchJobName + "-" + jobToken; ; } else { diff --git a/src/SqlBuildManager.Console/Program.cs b/src/SqlBuildManager.Console/Program.cs index bae9d35d..498c19f0 100644 --- a/src/SqlBuildManager.Console/Program.cs +++ b/src/SqlBuildManager.Console/Program.cs @@ -805,6 +805,12 @@ internal async static Task QueueOverrideTargets(CommandLineArgs cmdLine) log.LogError("A --servicebusconnection value is required. Please include this in either the settings file content or as a specific command option"); return 9839; } + (int ret, string msg) = Validation.ValidateBatchjobName(cmdLine.BatchArgs.BatchJobName); + if(ret != 0) + { + log.LogError(msg); + return ret; + } int tmpValReturn = Validation.ValidateAndLoadMultiDbData(cmdLine.MultiDbRunConfigFileName, cmdLine, out MultiDbData multiData, out string[] errorMessages); if (tmpValReturn != 0) diff --git a/src/SqlBuildManager.Console/Queue/QueueManager.cs b/src/SqlBuildManager.Console/Queue/QueueManager.cs index 5956ee2a..f60699db 100644 --- a/src/SqlBuildManager.Console/Queue/QueueManager.cs +++ b/src/SqlBuildManager.Console/Queue/QueueManager.cs @@ -178,11 +178,16 @@ private async Task> GetCountBasedTargetsFromQueu } return lstMsg; } - private async Task> GetSessionBasedTargetsFromQueue(int maxMessages, bool resetSession) + + + private async Task> GetSessionBasedTargetsFromQueue(int maxMessages, bool resetSession, int retry = 0) { var lstMsg = new List(); + bool foundMessages = false; try { + + //Init the receiver and try to acquire a session if (_sessionReceiver == null || resetSession) { log.LogInformation("Attempting to get new queue session for next Server..."); @@ -191,30 +196,70 @@ private async Task> GetSessionBasedTargetsFromQu try { _sessionReceiver = await this.Client.AcceptNextSessionAsync(this.topicName, this.topicSessionSubscriptionName, new ServiceBusSessionReceiverOptions() { ReceiveMode = ServiceBusReceiveMode.PeekLock }, token); - }catch(TaskCanceledException) + } + catch(TaskCanceledException) { - return lstMsg; + return lstMsg ; } } var messages = await _sessionReceiver.ReceiveMessagesAsync(maxMessages, new TimeSpan(0, 0, 10)); + + //If no messages in the current session, try to acquire a new session if (messages.Count == 0 && resetSession == false) { return await GetSessionBasedTargetsFromQueue(maxMessages, true); } - foreach (var message in messages) + else { - if (message.Subject.ToLower().Trim() != batchJobName.ToLower().Trim()) - { - log.LogWarning($"Message {message.MessageId} has incorrect Batch Job name"); - await this.MessageReceiver.DeadLetterMessageAsync(message); - log.LogWarning($"Send message '{message.MessageId} to deadletter. Subject of '{message.Subject}' did not match batch job name of '{batchJobName}'"); - } - else + foundMessages = true; + } + + //Got messages, not try to see if they are a match for the current job, if not, deadletter. Keep looking until some for the current job are found + if (foundMessages) + { + while (foundMessages || lstMsg.Count() == 0) { - lstMsg.Add(message); + foreach (var message in messages) + { + if (message.Subject.ToLower().Trim() != batchJobName.ToLower().Trim()) + { + log.LogWarning($"Message {message.MessageId} has incorrect Batch Job name '{batchJobName}'"); + try + { + await _sessionReceiver.DeadLetterMessageAsync(message); + log.LogWarning($"Send message '{message.MessageId}' to deadletter. Subject of '{message.Subject}' did not match batch job name of '{batchJobName}'"); + } + catch (Exception exe) + { + log.LogWarning($"Failed to deadletter message '{message.MessageId}': {exe.Message}"); + } + } + else + { + lstMsg.Add(message); + } + } + + //if they all got deadlettered, try to get some more, until there are none left! + if(lstMsg.Count == 0) + { + messages = await _sessionReceiver.ReceiveMessagesAsync(maxMessages, new TimeSpan(0, 0, 10)); + if(messages.Count == 0) + { + foundMessages = false; + } + } + else + { + return lstMsg; + } } } + else + { + return lstMsg; + } } catch (ServiceBusException sbe) { @@ -228,7 +273,12 @@ private async Task> GetSessionBasedTargetsFromQu case ServiceBusFailureReason.MessageLockLost: log.LogError($"Lock lost for message! There may be a issue with the messages in the topic: '{this.topicSessionSubscriptionName}"); - break; + if(retry == 5) + { + throw; + } + return await GetSessionBasedTargetsFromQueue(maxMessages, true, retry++); + default: throw; } diff --git a/src/SqlBuildManager.Console/Threaded/Concurrency.cs b/src/SqlBuildManager.Console/Threaded/Concurrency.cs index 92bdee5a..b771027c 100644 --- a/src/SqlBuildManager.Console/Threaded/Concurrency.cs +++ b/src/SqlBuildManager.Console/Threaded/Concurrency.cs @@ -111,7 +111,7 @@ public class Concurrency //Special case... is the number of buckets close to the number of servers? If so, do minumum consolidation var gap = Math.Abs((consolidated.Count() + buckets.Count()) - fixedBucketCount); - if (gap <= 6 && fixedBucketCount / gap > 2) + if (gap != 0 && gap <= 6 && fixedBucketCount / gap > 2) { //Combine the smallest buckets until we hit the fixed bucket count while(buckets.Count() > 0 && consolidated.Count() + buckets.Count() > fixedBucketCount) diff --git a/src/SqlBuildManager.Console/Validation.cs b/src/SqlBuildManager.Console/Validation.cs index 439e28d5..76b9a27c 100644 --- a/src/SqlBuildManager.Console/Validation.cs +++ b/src/SqlBuildManager.Console/Validation.cs @@ -374,10 +374,29 @@ public static int ValidateBatchArguments(CommandLineArgs cmdLine, out string[] e returnVal = -888; } + (int ret, string msg) = ValidateBatchjobName(cmdLine.BatchArgs.BatchJobName); + if(ret != 0) + { + messages.Add(msg); + returnVal = ret; + } + errorMessages = messages.ToArray(); return returnVal; } + public static (int, string) ValidateBatchjobName(string batchJobName) + { + if (!String.IsNullOrEmpty(batchJobName)) + { + if (batchJobName.Length < 3 || batchJobName.Length > 41 || !Regex.IsMatch(batchJobName, @"^[a-z0-9]+(-[a-z0-9]+)*$")) + { + return(-888, $"The value for --jobname must be: lower case, between 3 and 41 characters in length, and the only special character allowed are dashes '-'{Environment.NewLine}\tThis requirement is because the job name is also the storage container name and needs to accomodate a timestamp: https://docs.microsoft.com/en-us/rest/api/storageservices/Naming-and-Referencing-Containers--Blobs--and-Metadata"); + } + } + return (0, ""); + } + public static int ValidateBatchPreStageArguments(ref CommandLineArgs cmdLine, out string[] errorMessages) { int returnVal = 0; diff --git a/src/SqlBuildManager.Logging/ApplicationLogger.cs b/src/SqlBuildManager.Logging/ApplicationLogger.cs index 62904de6..7167763a 100644 --- a/src/SqlBuildManager.Logging/ApplicationLogger.cs +++ b/src/SqlBuildManager.Logging/ApplicationLogger.cs @@ -134,7 +134,7 @@ private set } } - internal static void CloseAndFlush() + public static void CloseAndFlush() { if (serilogLogger != null) { diff --git a/src/SqlSync/change_notes.html b/src/SqlSync/change_notes.html index 72ce73d4..0468919e 100644 --- a/src/SqlSync/change_notes.html +++ b/src/SqlSync/change_notes.html @@ -42,6 +42,8 @@

SQL Build Manager Change Notes

Version 13.0.1
FIXED: Updated distribution algorithm for `--concurrencytype` of `Server` and `MaxPerServer` when number of Batch nodes is very close to the number of SQL Server targets. Was yielding less than the number of nodes.
+
+ FIXED: Updated Service Bus message retrieval to better manage when messages not matching the job name are in large quantity

Version 13.0.0
diff --git a/src/SqlSync/change_notes.xml b/src/SqlSync/change_notes.xml index 242abd8c..90776447 100644 --- a/src/SqlSync/change_notes.xml +++ b/src/SqlSync/change_notes.xml @@ -2,6 +2,7 @@ Updated distribution algorithm for `--concurrencytype` of `Server` and `MaxPerServer` when number of Batch nodes is very close to the number of SQL Server targets. Was yielding less than the number of nodes. + Updated Service Bus message retrieval to better manage when messages not matching the job name are in large quantity New option to leverage Azure Service Bus Topic as a database target source. See the [Azure Batch](azure_batch.md) docs for more detail