workers: add landing worker warning logs #383
Conversation
zzzeid
commented
Mar 6, 2024
zzzeid
commented
- add warning log for large queue size (bug 1879896)
- add warning log for too many attempts (bug 1880310)
- add warning log for large queue size (bug 1879896) - add warning log for too many attempts (bug 1880310)
| for r in self.applicable_repos | ||
| if treestatus_subsystem.client.is_open(repo_clone_subsystem.repos[r].tree) | ||
| ] | ||
| logger.info(f"{len(self.enabled_repos)} enabled repos: {self.enabled_repos}") |
There was a problem hiding this comment.
Yes, I am not sure if it is a useful thing to have in the logs constantly, since this is basically the repos in the environment variable, minus any trees that are closed. Perhaps it makes more sense to add it as a warning instead, if the two don't match?
landoapi/workers/landing_worker.py
Outdated
| f"{len(self.applicable_repos)} applicable repos: {self.applicable_repos}" | ||
| def check_landing_worker_warnings(self): | ||
| """Log messages that show various important statistics about the landing worker.""" | ||
| TOO_MANY_ATTEMPTS_THRESHOLD = 10 |
There was a problem hiding this comment.
Move this to the class-level, or perhaps add it as a kwarg so we can override it.
landoapi/workers/landing_worker.py
Outdated
| """Log messages that show various important statistics about the landing worker.""" | ||
| TOO_MANY_ATTEMPTS_THRESHOLD = 10 | ||
| # TODO: should this threshold be different for try landing worker? | ||
| QUEUE_SIZE_THRESHOLD = 20 |
There was a problem hiding this comment.
Same with this threshold variable.
landoapi/workers/landing_worker.py
Outdated
| LandingJob.status.in_(ACTIVE_STATUSES) | ||
| ).count() | ||
| if queue_size >= QUEUE_SIZE_THRESHOLD: | ||
| logger.warning(f"The landing queue size of {queue_size} exceeds threshold") |
There was a problem hiding this comment.
Add the threshold size here like f"... exceeds threshold {QUEUE_SIZE_THRESHOLD}."
landoapi/workers/landing_worker.py
Outdated
| ACTIVE_STATUSES = ( | ||
| LandingJobStatus.SUBMITTED, | ||
| LandingJobStatus.DEFERRED, | ||
| LandingJobStatus.IN_PROGRESS, | ||
| ) |
There was a problem hiding this comment.
This tuple is a copy of the applicable_statuses variable in job_queue_query. Let's move them to a variable where they can be shared (maybe a class-level LandingJobStatus.active_statuses or something along those lines).
| if runaway_jobs.count() > 0: | ||
| job = runaway_jobs.all()[0] | ||
| logger.warning( | ||
| f"Active landing job ({job}) has too many attempts ({job.attempts})" | ||
| ) |
There was a problem hiding this comment.
Should we cancel the job in that case?
There was a problem hiding this comment.
That would be a good idea, but I think it would be outside the scope of this PR since it would require us to also send a notification email. Maybe it can be done as a follow up.
