-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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 general performance for a variety of high-load job launch use cases #8403
Merged
softwarefactory-project-zuul
merged 2 commits into
ansible:devel
from
chrismeyersfsu:fix-same_jt_abuse_devel
Oct 19, 2020
Merged
Improve general performance for a variety of high-load job launch use cases #8403
softwarefactory-project-zuul
merged 2 commits into
ansible:devel
from
chrismeyersfsu:fix-same_jt_abuse_devel
Oct 19, 2020
Conversation
This file contains 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
Build succeeded.
|
awx/main/models/unified_jobs.py
Outdated
# This dodges lock contention at the expense of the foreign key not being | ||
# completely correct. | ||
if getattr(self, 'allow_simultaneous', False): | ||
connection.on_commit(self._update_parent_instance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary if self.status == status_before:
?
Would this be better?
if self.status != status_before:
if getattr(self, 'allow_simultaneous', False):
connection.on_commit(self._update_parent_instance)
else:
self._update_parent_instance()
* We update the parent unified job template to point at new jobs created. We also update a similar foreign key when the job finishes running. This causes lock contention when the job template is allow_simultaneous and there are a lot of jobs from that job template running in parallel. I've seen as bad as 5 minutes waiting for the lock when a job finishes. * This change moves the parent->child update to OUTSIDE of the transaction if the job is allow_simultaneous (inherited from the parent unified job). We sacrafice a bit of correctness for performance. The logic is, if you are launching 1,000 parallel jobs do you really care that the job template contains a pointer to the last one you launched? Probably not. If you do, you can always query jobs related to the job template sorted by created time.
* Do not query the database for the set of Instance that belong to the group for which we are trying to fit a job on, for each job. * Instead, cache the set of instances per-instance group.
chrismeyersfsu
force-pushed
the
fix-same_jt_abuse_devel
branch
from
October 19, 2020 14:56
7b0ce7a
to
2eac5a8
Compare
Build succeeded.
|
ryanpetrello
approved these changes
Oct 19, 2020
fosterseth
approved these changes
Oct 19, 2020
ryanpetrello
changed the title
Fix same jt abuse devel
Improve performance for a variety of high-load job launch use cases
Oct 19, 2020
ryanpetrello
changed the title
Improve performance for a variety of high-load job launch use cases
Improve general performance for a variety of high-load job launch use cases
Oct 19, 2020
regate |
Build succeeded (gate pipeline).
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
reduce per-job database query count
Do not query the database for the set of Instance that belong to the
group for which we are trying to fit a job on, for each job.
Instead, cache the set of instances per-instance group.
reduce parent->child lock contention
We update the parent unified job template to point at new jobs
created. We also update a similar foreign key when the job finishes
running. This causes lock contention when the job template is
allow_simultaneous and there are a lot of jobs from that job template
running in parallel. I've seen as bad as 5 minutes waiting for the lock
when a job finishes.
This change moves the parent->child update to OUTSIDE of the
transaction if the job is allow_simultaneous (inherited from the parent
unified job). We sacrafice a bit of correctness for performance. The
logic is, if you are launching 1,000 parallel jobs do you really care
that the job template contains a pointer to the last one you launched?
Probably not. If you do, you can always query jobs related to the job
template sorted by created time.