Skip to content

Attempt to fix bottlenecks and performance related to Facade#399

Open
MoralCode wants to merge 6 commits into
mainfrom
bug/facade_perf
Open

Attempt to fix bottlenecks and performance related to Facade#399
MoralCode wants to merge 6 commits into
mainfrom
bug/facade_perf

Conversation

@MoralCode

Copy link
Copy Markdown
Contributor

Description
This PR attempts to fix a couple non-normal conditions with regard to facade.

  1. An instance that adds a lot of new repos at once can see existing repo recollections fall behind the configured window (because new repo collection currently takes priority)
  2. an instance with a lot of repos that have collection errors can become severely bottlenecked and have keys potentially exhausted when ALL errored repos are reset to non-error states once a day by the error retry task.

These two issues can combine to grind collection to a halt and waste significant resources.

This PR addresses these issues by:

  1. reserving half (or slightly less than half) of available collection slots for new collection (if needed), ensuring recollection jobs still have a path through the pipeline.
  2. adjusting the error retry task so that it only resets errored repos when there are no new repos to collect.

This PR fixes #391

Notes for Reviewers

I still have yet to test this fully. itll probably be complicated to test

Signed commits

  • Yes, I signed my commits.

Generative AI disclosure

Please select one option:

  • This contribution was NOT assisted or created by Generative AI tools.
  • This contribution was assisted or created by Generative AI tools.

If AI tools were used, please provide details below:
- What tools were used? Sonnet 4.6 Medium via Cursor
- How were these tools used? lots of back and forth to attempt to diagnose the general collection performance issues. Some AI code completion was used to write the code
- Did you review these outputs before submitting this PR? Yes, at all times I was making sure the AI tool's diagnosis was sensible given my knowledge of the code and all written code was looked at and understood by me.

f"""UPDATE collection_status SET ml_status = '{CollectionState.PENDING.value}'"""
f""" WHERE ml_status = '{CollectionState.ERROR.value}' and ml_data_last_collected is NULL;"""

with DatabaseSession(logger, engine) as session:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes i know this part of the code needs a rewrite

@@ -2,6 +2,7 @@
import logging
import random

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[pylint] reported by reviewdog 🐶
W0611: Unused import random (unused-import)

Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
Signed-off-by: Adrian Edwards <adredwar@redhat.com>
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.

Adding lots of new repos at once can create perceived bottlenecks

2 participants