Restart summarizer when it stops [0.10] #348
Conversation
arinwt
changed the title
| }, | ||
| (error) => { | ||
| this.logger.sendErrorEvent({ eventName: "RunningSummarizerFailed" }, error); | ||
| runSummarizerEvent.cancel({}, error); | ||
| this.computeSummarizer(this.connected); |
There was a problem hiding this comment.
Feels like this should be in finally case.
There was a problem hiding this comment.
I agree but I was trying to avoid floating promises and I wanted the compute summarizer call to occur after the telemetry event.
| } | ||
|
|
||
| // if we are connected and the elected summarizer client, start a summarizer | ||
| if (this.shouldSummarize && !this.runningSummarizer) { |
There was a problem hiding this comment.
!this.runningSummarizer [](start = 35, length = 24)
It feels like this code asks either for assert(!this.runningSummarizer) or handling of when it's running - stopping it if !this.shouldSummarize
There was a problem hiding this comment.
I think an assert works here, since we should have already been stopped above.
| } | ||
|
|
||
| if (this.runningSummarizer) { | ||
| summarizer.stop("parentAlreadyRunningSummarizer"); |
There was a problem hiding this comment.
summarizer.stop("parentAlreadyRunningSummarizer"); [](start = 12, length = 50)
Sounds like this is an error, i.e. it would be nice to have explicit error telemetry events here.
|
I want to spend more time on SummaryManager in a later PR, but I think we should still get this PR merged to reduce Summary Nacks. |
* Restart summarizer when it completes and do not run new summarizer if already running (#348) * Container handle errors during reload context [0.10] (#369) * Handle errors during reloadContext and await pause/resume calls * Always emit errors and reject promise on error * Change reloadContext to reject throw error on catch * Refactor SummaryManager [0.10] (#362) * Change SummaryManager to state machine * Remove shouldNotStop telemetry event * Clean up switch statement to prevent fallthrough * Small changes for PR feedback * Use finally for run completion * Summarize more frequently [0.10] (#371) * Quorum ops count towards summarizer heuristics; summarizer handle ops while summarizing or pending * Ignore summarizer leave messages, just in case * Refactor summarizer code * Add telemetry and small refactors * Add comments; small refactor of summary op handling; track seq instead of flag
Also prevent starting a new summarizer if one is already running and add a few telemetry details.
Fixes #107.