Skip to content

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
warwick/pattern-timeproviderfrom
warwick/retry-start-time
Open

Record a start time for every retry operation, and keep SQL Server statements under the parameter limit#5846
warwickschroeder wants to merge 4 commits into
warwick/pattern-timeproviderfrom
warwick/retry-start-time

Conversation

@warwickschroeder

Copy link
Copy Markdown
Contributor

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

  • Only a group retry announces itself, and announcing is what sets the start time. Wait is called for RetryType.FailureGroup alone (FailureGroupsRetryController.cs:37, RetryAllInGroupHandler.cs:39); every other type goes straight to Prepare, which never touched Started, so StoreHistoryHandler wrote default(DateTime) into the retry history the recoverability screen reads.
  • The batch document had the right time on the line that dropped it. RetriesGateway passed the clock to store.CreateBatch and called Preparing without it, so the two agreed again only after a restart, when RebuildRetryOperationState reads the time back off the batch.
  • A bulk retry threw away its own description. ProcessRequest held the request's Originator, the text that names the operation ("all messages for endpoint Sales"), and did not pass it to Preparing.
  • A leftover skip count makes an operation miss its own finish line. Prepare reset the forwarded and prepared counts but not NumberOfMessagesSkipped, so a second run of the same operation id overshoots the total CheckForCompletion compares against and sits on Forwarding for ever.
  • One failure marks every later run as failed. Fail() fires from RetryDocumentManager.cs:38 for orphaned batches and Prepare never cleared it, so every subsequent run of that request id reported IsFailed, went to history as a failure and counted as one in the metrics. Group retries escaped both carry-overs because Wait clears the counters.
  • The SQL Server chunk sizes divided 2100 exactly, and 2100 is not the whole budget. sp_executesql spends two of the slots on the statement text and the parameter list, so 2100 / columns sent 2102 and any full chunk was rejected outright: 525 rows for InsertGroups, 420 for InsertMissingKnownEndpoints, 700 for InsertMissingRetryClaims.
  • ResolveRetriedMessages also 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.Preparing takes the start time and, optionally, the originator, and InMemoryRetry.Prepare stamps them.
  • Whoever asks for the retry supplies the time, once. RetriesGateway reads the clock into startedAt and gives the same value to Preparing and store.CreateBatch; the bulk route passes request.StartTime and request.Originator.
  • Wait still wins wherever it ran. Prepare stamps only when the operation is beginning a new run or was never stamped, so a group keeps what Wait gave it.
  • A re-run starts clean. Entering Prepare from Completed also clears NumberOfMessagesSkipped, CompletionTime and Failed.
  • The SQL Server budget is 2098, less whatever the statement spends on itself. MaxRowsPerStatement(columns, sharedParameters) subtracts a named ExecuteSqlOverhead of 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.
  • Deliberately unchanged: PostgreSQL's flat 50 rows. It accepts 65535 bind parameters and the widest statement uses 1350, so the 50 is not a ceiling but a chunk that keeps the generated text to one full shape plus a remainder for the planner to cache, and a comment now says so.

Test coverage

  • A retry that never waited records a real start time, through the API. The RavenDB acceptance test for POST /api/errors/retry now reads the history row back, and fails on 01 Jan 0001.
  • The operation and the batch agree on when it started, which is what a restart rebuilds the start time from. New persistence test, on RavenDB, SQL Server and PostgreSQL.
  • The bulk route keeps the request's own time and title rather than the wall clock. Same three stores.
  • Wait survives Prepare, and a re-run reports the later start. Three unit tests in RetryStartTimeTests, plus two existing group tests that now pin Started to what Wait set.
  • Each carry-over defect has a test named after it: a second run inherits neither the first run's skip count nor its failure.
  • Four dialect tests overflow a chunk on purpose, at 200 upserts, 600 groups, 500 known endpoints and 1100 confirmations, against real SQL Server and PostgreSQL.
  • The shared persistence suite now runs whole on all three stores. RetryStateTests and RetryConfirmationProcessorTests were the last two exclusions; making them store-agnostic meant real GUIDs and a seeded processing attempt, plus two app.config files so Settings stops throwing.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant