This repository was archived by the owner on Sep 30, 2024. It is now read-only.
worker: Reduce frequency of very frequently run jobs#62864
Merged
Conversation
I looked at background routines and noticed that many routines are running _very_ frequently, which can cause higher than necessary load, and also stress on redis to store all these runs. I tried to put in some more sensible numbers (aligning it with what most other routines seem to use). For many instances, this will mean a 30x reduction in job invocations. Test plan: Code review from owners, and will watch S2 and dotcom for potential issues.
Member
Author
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
eseliger
commented
May 22, 2024
| func (c *config) Load() { | ||
| c.EncryptionInterval = c.GetInterval("RECORD_ENCRYPTER_INTERVAL", "1s", "How frequently to encrypt/decrypt a batch of records in the database.") | ||
| c.MetricsInterval = c.GetInterval("RECORD_ENCRYPTER_METRICS_INTERVAL", "10s", "How frequently to update progress metrics related to encryption/decryption.") | ||
| c.EncryptionInterval = c.GetInterval("RECORD_ENCRYPTER_INTERVAL", "10s", "How frequently to encrypt/decrypt a batch of records in the database.") |
Member
Author
There was a problem hiding this comment.
This is only relevant for when encryption is initially enabled, or getting disabled. Records written while encryption is on will be encrypted right away. This should be more than sensible.
eseliger
commented
May 22, 2024
| c.EncryptionInterval = c.GetInterval("RECORD_ENCRYPTER_INTERVAL", "1s", "How frequently to encrypt/decrypt a batch of records in the database.") | ||
| c.MetricsInterval = c.GetInterval("RECORD_ENCRYPTER_METRICS_INTERVAL", "10s", "How frequently to update progress metrics related to encryption/decryption.") | ||
| c.EncryptionInterval = c.GetInterval("RECORD_ENCRYPTER_INTERVAL", "10s", "How frequently to encrypt/decrypt a batch of records in the database.") | ||
| c.MetricsInterval = c.GetInterval("RECORD_ENCRYPTER_METRICS_INTERVAL", "30s", "How frequently to update progress metrics related to encryption/decryption.") |
Member
Author
There was a problem hiding this comment.
Just metrics, should be fine to lag a bit more behind.
eseliger
commented
May 22, 2024
|
|
||
| func (c *Config) Load() { | ||
| c.Interval = c.GetInterval("CODEINTEL_RANKING_COORDINATOR_INTERVAL", "1s", "How frequently to run the ranking coordinator.") | ||
| c.Interval = c.GetInterval("CODEINTEL_RANKING_COORDINATOR_INTERVAL", "30s", "How frequently to run the ranking coordinator.") |
Member
Author
There was a problem hiding this comment.
This and the following ranking configs are not doing any work on instances as the feature is disabled everywhere, but we still run the background routines, so this will reduce frequency but not affect any functionality.
eseliger
commented
May 22, 2024
|
|
||
| func (c *Config) Load() { | ||
| c.Interval = c.GetInterval("CODEINTEL_UPLOAD_BACKFILLER_INTERVAL", "10s", "The frequency with which to run periodic codeintel backfiller tasks.") | ||
| c.Interval = c.GetInterval("CODEINTEL_UPLOAD_BACKFILLER_INTERVAL", "30s", "The frequency with which to run periodic codeintel backfiller tasks.") |
Member
Author
There was a problem hiding this comment.
Does this seem sensible to Graph?
eseliger
commented
May 22, 2024
| commitGraphUpdateTaskInterval := env.ChooseFallbackVariableName("CODEINTEL_UPLOAD_COMMITGRAPH_UPDATE_TASK_INTERVAL", "PRECISE_CODE_INTEL_COMMIT_GRAPH_UPDATE_TASK_INTERVAL") | ||
|
|
||
| c.Interval = c.GetInterval("CODEINTEL_UPLOAD_COMMITGRAPH_UPDATER_INTERVAL", "1s", "How frequently to run the upload commitgraph updater routine.") | ||
| c.Interval = c.GetInterval("CODEINTEL_UPLOAD_COMMITGRAPH_UPDATER_INTERVAL", "10s", "How frequently to run the upload commitgraph updater routine.") |
Member
Author
There was a problem hiding this comment.
Does this seem sensible to Graph?
eseliger
commented
May 22, 2024
|
|
||
| c.CommitBatchSize = c.GetInt(commitBatchSize, "100", "The number of commits to process per upload at a time.") | ||
| c.ExpirerInterval = c.GetInterval("CODEINTEL_UPLOAD_EXPIRER_INTERVAL", "1s", "How frequently to run the upload expirer routine.") | ||
| c.ExpirerInterval = c.GetInterval("CODEINTEL_UPLOAD_EXPIRER_INTERVAL", "30s", "How frequently to run the upload expirer routine.") |
Member
Author
There was a problem hiding this comment.
Does this seem sensible to Graph?
keynmol
approved these changes
May 23, 2024
keynmol
left a comment
Contributor
There was a problem hiding this comment.
I think all of these are fine - my guess is that all those 1s are remnants from the time when the feature was being developed, where local feedback loop needed to be tight.
Don't a see a reason for any of those frequencies to be 1s.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
I looked at background routines and noticed that many routines are running very frequently, which can cause higher than necessary load, and also stress on redis to store all these runs.
I tried to put in some more sensible numbers (aligning it with what most other routines seem to use). For many instances, this will mean a 30x reduction in job invocations.
Test plan:
Code review from owners, and will watch S2 and dotcom for potential issues.