Record a start time for every retry operation, and keep SQL Server statements under the parameter limit - #5846
Open
warwickschroeder wants to merge 4 commits into
Open
Conversation
… titles. - add remaining database agnostic tests.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Every retry now records when it was asked for, and SQL Server statements stay inside the parameter limit
ServiceControl keeps a live picture of each retry operation in memory and writes a row to the retry history when it finishes. Only a failure-group retry announced itself before starting, and announcing is what set the start time and the description, so every other kind of retry finished with a start time of 01 January 0001 and nothing to name it. This branch stamps both when the retry is asked for, clears state that leaked from one run of an operation into the next, and corrects the SQL Server chunk arithmetic that had four bulk statements asking for more parameters than the server accepts.
What is wrong today
Waitis called forRetryType.FailureGroupalone (FailureGroupsRetryController.cs:37,RetryAllInGroupHandler.cs:39); every other type goes straight toPrepare, which never touchedStarted, soStoreHistoryHandlerwrotedefault(DateTime)into the retry history the recoverability screen reads.RetriesGatewaypassed the clock tostore.CreateBatchand calledPreparingwithout it, so the two agreed again only after a restart, whenRebuildRetryOperationStatereads the time back off the batch.ProcessRequestheld the request'sOriginator, the text that names the operation ("all messages for endpoint Sales"), and did not pass it toPreparing.Preparereset the forwarded and prepared counts but notNumberOfMessagesSkipped, so a second run of the same operation id overshoots the totalCheckForCompletioncompares against and sits onForwardingfor ever.Fail()fires fromRetryDocumentManager.cs:38for orphaned batches andPreparenever cleared it, so every subsequent run of that request id reportedIsFailed, went to history as a failure and counted as one in the metrics. Group retries escaped both carry-overs becauseWaitclears the counters.sp_executesqlspends two of the slots on the statement text and the parameter list, so2100 / columnssent 2102 and any full chunk was rejected outright: 525 rows forInsertGroups, 420 forInsertMissingKnownEndpoints, 700 forInsertMissingRetryClaims.ResolveRetriedMessagesalso carries@p0, "now" for the whole statement on top of the two parameters per row, so its 1050-row chunk arrived as 2103.What it looks like afterwards
RetryingManager.Preparingtakes the start time and, optionally, the originator, andInMemoryRetry.Preparestamps them.RetriesGatewayreads the clock intostartedAtand gives the same value toPreparingandstore.CreateBatch; the bulk route passesrequest.StartTimeandrequest.Originator.Waitstill wins wherever it ran.Preparestamps only when the operation is beginning a new run or was never stamped, so a group keeps whatWaitgave it.PreparefromCompletedalso clearsNumberOfMessagesSkipped,CompletionTimeandFailed.MaxRowsPerStatement(columns, sharedParameters)subtracts a namedExecuteSqlOverheadof 2 first: groups 525 to 524, endpoints 420 to 419, retry claims 700 to 699, confirmations 1050 to 1048, and the 27-column upsert unchanged at 77.Test coverage
POST /api/errors/retrynow reads the history row back, and fails on01 Jan 0001.WaitsurvivesPrepare, and a re-run reports the later start. Three unit tests inRetryStartTimeTests, plus two existing group tests that now pinStartedto whatWaitset.RetryStateTestsandRetryConfirmationProcessorTestswere the last two exclusions; making them store-agnostic meant real GUIDs and a seeded processing attempt, plus twoapp.configfiles soSettingsstops throwing.