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 speed and memory performance of syncing #440
Conversation
https://pulp.plan.io/issues/5688 Required PR: pulp/pulpcore#440 closes #5688
https://pulp.plan.io/issues/5688 Required PR: pulp/pulpcore#440 closes #5688
https://pulp.plan.io/issues/5688 Required PR: pulp/pulpcore#440 closes #5688
| @@ -71,3 +71,20 @@ def get_view_name_for_model(model_obj, view_action): | |||
| if registered_viewset is viewset: | |||
| return '-'.join((base_name, view_action)) | |||
| raise LookupError('view not found') | |||
|
|
|||
|
|
|||
| def batch_qs(qs, batch_size=1000): | |||
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.
| @@ -496,6 +500,9 @@ def remove_content(self, content): | |||
| if self.complete: | |||
| raise ResourceImmutableError(self) | |||
|
|
|||
| if not content or not content.count(): | |||
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.
What motivated adding this?
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.
SQLAlchemy enforces to not pass an empty list due performance problems: https://developpaper.com/query-empty-list-problem-analysis-of-sql-alchemy-in/
So I did it to see how it would affect performance on django orm, and I notice a 300 seconds difference from my previous test without this.
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.
Yup that makes sense. Thank you.
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.
This looks great. Thank you so much! Great job.
💯
https://pulp.plan.io/issues/5688 Required PR: pulp/pulpcore#440 closes #5688
https://pulp.plan.io/issues/5688 Required PR: pulp/pulpcore#440 closes #5688
| @@ -15,7 +15,7 @@ | |||
| _logger = logging.getLogger(__name__) | |||
|
|
|||
| # number of ms between save() calls when _using_context_manager is set | |||
| BATCH_INTERVAL = 500 | |||
| BATCH_INTERVAL = 2000 | |||
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.
@gerrod3 @bmbouter @daviddavis maybe we need to change these numbers for pulp_ansible sync ?
| @@ -81,7 +81,7 @@ async def run(self): | |||
| log.debug(_('%(name)s - next: %(content)s.'), {'name': self, 'content': content}) | |||
| yield content | |||
|
|
|||
| async def batches(self, minsize=50): | |||
| async def batches(self, minsize=500): | |||
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.
@gerrod3 @bmbouter @daviddavis maybe we need to change these numbers for pulp_ansible sync?
| @@ -163,7 +163,7 @@ def __str__(self): | |||
| return '[{id}] {name}'.format(id=id(self), name=self.__class__.__name__) | |||
|
|
|||
|
|
|||
| async def create_pipeline(stages, maxsize=100): | |||
| async def create_pipeline(stages, maxsize=1000): | |||
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.
@gerrod3 @bmbouter @daviddavis maybe we need to change these numbers for pulp_ansible sync?
https://pulp.plan.io/issues/5688
ref #5688
Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/en/3.0/nightly/contributing/pull-request-walkthrough.html