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

move clientCron onto a separate timer #1387

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from

Conversation

JimB123
Copy link
Contributor

@JimB123 JimB123 commented Dec 3, 2024

The serverCron() function contains a variety of maintenance functions and is set up as a timer job, configured to run at a certain rate (hz). The default rate is 10hz (every 100ms).

One of the things that serverCron() does is to perform maintenance functions on connected clients. Since the number of clients is variable, and can be very large, this could cause latency spikes when the 100ms serverCron() task gets invoked. To combat those latency spikes, a feature called "dynamic-hz" was introduced. This feature will run serverCron() more often, if there are more clients. The clients get processed up to 200 at a time. The delay for serverCron() is shortened with the goal of processing all of the clients every second.

The result of this is that some of the other (non-client) maintenance functions also get (unnecessarily) run more often. Like cronUpdateMemoryStats() and databasesCron(). Logically, it doesn't make sense to run these functions more often, just because we happen to have more clients attached.

This PR separates client activities onto a separate, variable, timer. The "dynamic-hz" feature is eliminated. Now, serverCron will run at a standard configured rate. The separate clients cron will automatically adjust based on the number of clients. This has the added benefit that often, the 2 crons will fire during separate event loop invocations and will usually avoid the combined latency impact of doing both maintenance activities together.

The new timer follows the same rules which were established with the dynamic HZ feature.

  • The goal is to process all of the clients once per second
  • We never want to process more than 200 clients in a single invocation (MAX_CLIENTS_PER_CLOCK_TICK)
  • We always process at least 5 clients at a time (CLIENTS_CRON_MIN_ITERATIONS)
  • The minimum rate is determined by HZ

The delay (ms) for the new timer is also more precise, computing the number of milliseconds needed to achieve the goal of reaching all of the clients every second. The old dynamic-hz feature just performs a doubling of the HZ until the clients processing rate is achieved (i.e. delays of 100ms, 50ms, 25ms, 12ms...)

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