Skip to content

Ab/refactor healthcheck - #3760

Open
abeglova wants to merge 7 commits into
mainfrom
ab/refactor-healthcheck
Open

Ab/refactor healthcheck#3760
abeglova wants to merge 7 commits into
mainfrom
ab/refactor-healthcheck

Conversation

@abeglova

@abeglova abeglova commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

closes https://github.com/mitodl/hq/issues/12792

Description (What does it do?)

this pr breaks the embedding healthcheck into many restartable jobs to make it resilient to pod culling

How can this be tested?

make sure you have missing summaries, content file embeddings and resource embeddings

set SENTRY_DSN to the value from rc
set EMBEDDINGS_HEALTHCHECK_ALERT_CAP to 2 - this prevents local/rc environments from spamming sentry since alerts are per job now

from the shell run

from vector_search.tasks import embeddings_healthcheck
embeddings_healthcheck.delay()

visit the sentry dashboard for the "dev" environment

you should see the following warnings:
Warning: missing content file summaries detected
Warning: learning resources are missing their embeddings
Warning: learning resources are missing their embeddings
Warning: healthcheck alerts suppressed after reaching the per-run cap
Warning: content files are missing embeddings
Warning: content files are missing embeddings
Warning: healthcheck alerts suppressed after reaching the per-run cap

@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown

OpenAPI Changes

No changes detected

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@abeglova
abeglova marked this pull request as ready for review August 12, 2026 23:59
Copilot AI balanced review requested due to automatic review settings August 12, 2026 23:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors the Qdrant embeddings healthcheck for batched, parallel execution with capped Sentry reporting.

Changes:

  • Splits healthchecks into resource, content-file, and summary tasks.
  • Optimizes Qdrant existence checks.
  • Adds alert-cap configuration and expanded tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
vector_search/utils.py Avoids fetching unused Qdrant payloads and vectors.
vector_search/tasks.py Adds batched healthcheck dispatch and capped alerts.
vector_search/tasks_test.py Expands healthcheck and alert-cap coverage.
main/settings.py Configures the per-run Sentry alert cap.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread vector_search/tasks.py
Comment on lines +640 to +642
.filter(
Q(run__learning_resource__in=resources) | Q(learning_resource__in=resources)
)

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.

there arn't any content files whose serializer owner is only direct_learning_resource currently

Comment thread vector_search/tasks.py
Comment on lines +871 to +884
try:
return cache.incr(key)
except ValueError: # key absent
# add() is atomic, so exactly one of the workers racing to create the counter
# wins it; the losers fall through to incr instead of each resetting it to 1
# and handing every racer the same count of 1
if cache.add(key, 1, HEALTHCHECK_ALERT_TTL):
return 1
try:
return cache.incr(key)
except ValueError:
# expired between add() and incr(): only reachable if this run outlives
# HEALTHCHECK_ALERT_TTL, in which case counting from 1 again is right
return 1

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.

that's fine - it's ok if we sometimes get 2x the cap messages , we just want to make sure we don't generate thousands of messages in non-production environments and kill the sentry message budget

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.

2 participants