Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve DeploymentLoadPublisher logic and cleanup #8472

Merged

Conversation

ReubenBond
Copy link
Member

@ReubenBond ReubenBond commented Jun 6, 2023

  • UpdateRuntimeStatistics is a notification method and since we use it with Task.WhenAll, waiting for its completion before continuing can result in one (eg, dead) silo delaying others from receiving updates: a slow silo will prevent the next round of updates from being collected and sent. Additionally, we do not care if this method fails, so we do not need to track its completion. Therefore, I have marked the method [OneWay].
  • Statistics are ignored if they are not the latest, but we are using DateTime.UtcNow, so we are susceptible to clock changes. Setting the clock on a host backwards can prevent other hosts from applying updates for some period of time. Therefore, I changed the logic to use a monotonic clock which always proceeds on every cycle, even if the clock has been set backwards.
  • Currently we propagate local silo statistics to the local silo using the same mechanism used for remote silos, but this is a waste of a grain call. Instead, this PR applies the local statistics directly and skips sending them via RPC.
  • I made some small efficiency (reduced task allocs) and readability/debuggability (use async-await over ContinueWith) changes, plus a more invasive refactoring to rename members to fit the modern C# style.
Microsoft Reviewers: Open in CodeFlow

@benjaminpetit benjaminpetit merged commit bfc50a3 into dotnet:main Jun 13, 2023
18 checks passed
@ReubenBond ReubenBond deleted the enhance/deployment-load-publisher/1 branch June 13, 2023 15:10
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants