perf(commands): throttle progress-message DB writes to once per second per command - #266
Open
jordanfelle wants to merge 2 commits into
Open
jordanfelle wants to merge 2 commits into
jordanfelle wants to merge 2 commits into
Conversation
…ond per command
ProgressMessageTarget turns every progress log line ("Checking Info for <book>") into
CommandQueueManager.SetMessage, which did a synchronous UPDATE of Message + LastProgressAt
on the command row. An author refresh emits one per book, so a refresh spent a database
round trip per book just to change display text. Stack samples of a live bulk refresh had
this in ~17% of samples (LogProgress -> SetMessage -> SetFields -> UpdateFields).
Add ICommandQueueManager.SetProgressMessage (a default interface method that calls
SetMessage, so existing implementers and test doubles are untouched). The real manager keeps
the in-memory command current on every call - that is what the API reads while the command
runs - and persists at most once per second per command. ProgressMessageTarget now calls it.
Explicit messages (Completed/Failed/Cancelled/Resumed) still go through SetMessage: always
persisted, and they clear the command's throttle state. LastProgressAt already has its own
30s heartbeat (TouchProgress) and nothing else reads it, so its cadence is unchanged in effect.
Tests: at most one DB write for 200 progress messages, in-memory message always current,
SetMessage always persists and resets the throttle, throttling is per command. The
throttle tests fail with the interval set to 0.
Adversarial review of Chaptarr#266: the exact write-count assertions assumed the 1s throttle window never elapses mid-test. Use a 1-2 bound for the burst test and relative counts for the SetMessage/reset test.
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
1 participant
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.
Problem
ProgressMessageTargetforwards every progress log line toCommandQueueManager.SetMessage, which synchronouslyUPDATEsMessageandLastProgressAton the command row. An author refresh logs one line per book ("Checking Info for ..."), so a refresh paid one database round trip per book just to change display text.Found by sampling a live bulk refresh with
dotnet-stack(12 samples):RefreshBookService.LogProgress -> SetMessage -> SetFields -> UpdateFieldswas in 2 of 12 (~17%), alongside the per-book delete path (a separate change).Change
ICommandQueueManager.SetProgressMessage(command, message): a default interface method that callsSetMessage, so existing implementers and hand-written test doubles are unaffected.CommandQueueManageroverrides it: the in-memory command (what the API reads while it runs) is updated on every call; the DB write happens at most once per second per command.ProgressMessageTargetcallsSetProgressMessage.SetMessage: always persisted, and they clear that command's throttle state.LastProgressAtalready has its own 30s heartbeat (TouchProgress) and nothing else reads it.Tests
CommandQueueManagerProgressFixture(4 tests): one DB write for 200 progress messages; in-memory message always current;SetMessagealways persists and resets the throttle; throttling is per command. The throttle tests fail with the interval set to 0. FullChaptarr.Core.Test: 3039 passed.Not measured
Query-count change only; no before/after refresh timing yet.