Skip to content

fix(watson): fix + batch full-text index updates on Celery worker tasks#15172

Merged
mtesauro merged 3 commits into
DefectDojo:devfrom
valentijnscholten:fix/watson-batch-index-on-worker-tasks
Jul 9, 2026
Merged

fix(watson): fix + batch full-text index updates on Celery worker tasks#15172
mtesauro merged 3 commits into
DefectDojo:devfrom
valentijnscholten:fix/watson-batch-index-on-worker-tasks

Conversation

@valentijnscholten

@valentijnscholten valentijnscholten commented Jul 7, 2026

Copy link
Copy Markdown
Member

Description

Celery workers serve no HTTP request, so AsyncSearchContextMiddleware never opens a watson search_context. Without an active context, django-watson's post_save receiver falls into its else-branch and indexes every saved object synchronously — one watson_searchentry DELETE + INSERT per object. Any bulk save that runs in a worker (e.g. a large async import) then re-indexes objects one at a time, flooding the DB with thousands of index writes.

This PR refactors the performance test to perform a real API call. This results in queries and celery tasks triggered by middleware is now also captured. This explains the small increase in counts in this PR. But this actually a decrease because of the watson fix bundled in this PR.

Approach

Phased, to confirm the problem before fixing it (regression test added downstream in DefectDojo Pro, where the async-import worker path lives — see the paired Pro PR):

  1. Reproduce — a 1373-finding import through the worker path issued ~1000+ watson_searchentry INSERTs (query-count explosion). The failing test asserted the batched expectation.
  2. Fix — batch the worker-side index updates the same way the request path already does.

Changes

  • watson_search_context_for_task (dojo/middleware.py) — a context manager that opens a watson search_context for the duration of a Celery task so saves accumulate and drain in WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE-sized async batches on exit, mirroring AsyncSearchContextMiddleware for the request path. An empty context (task saved no indexed models) drains to nothing — a cheap no-op.
  • CELERY_TASK_CONTEXT_MANAGERS default (dojo/settings/settings.dist.py) — wires the CM in; PluggableContextTask enters it around every task. Downstreams (e.g. Pro) extend this list rather than replacing it.
  • Instance-bound intermediate-flush hook (dojo/middleware.py, dojo/tasks.py)install_intermediate_flush_hook() now binds the flush wrapper to the module-global search_context_manager instance instead of monkeypatching SearchContextManager at class level. The terminal index task update_watson_search_index_for_model builds its own local SearchContextManager to index one already-bounded batch; with the class-level patch, a full 1000-PK batch re-triggered the flush from inside the task and re-dispatched a clone of itself — infinite recursion under eager celery, an infinite re-dispatch loop (nothing ever indexed) on a real worker. Binding to the singleton makes that unrepresentable: watson's post_save and the request/task paths all go through the module-global instance (which flushes), while private throwaway contexts keep the stock add_to_context and simply index their batch on end(). The same loop is fixed on the release line by the minimal identity guard in fix(watson): only intermediate-flush the global search context singleton #15187 (extracted from perf(importers): add defer_product_grading option + harden watson index-flush hook #15165); this PR supersedes the guard on dev with the structural fix.

Test results

Verified in the integrated (Pro) environment; a downstream regression test drives the worker path (AsyncImporter) end-to-end:

  • 1373-finding JFrog import: watson_searchentry INSERTs ~1000+ → 14 (ceil(1000/100) + ceil(373/100)), with 1373/1373 findings still indexed exactly once (no findings dropped, no recursion).
  • unittests/test_watson_intermediate_flush.py passes (5/5), including the new test_ad_hoc_context_manager_does_not_drain regression test: an ad-hoc SearchContextManager pushed past the flush threshold must not drain (it would re-dispatch a clone of itself and loop). The existing tests exercise the global manager, which still flushes.
  • Loop verified fixed live: dispatching a full 1000-pk update_watson_search_index_for_model task on unguarded code self-replicates without bound (10 → 200+ tasks in ~3 min); on this branch the same dispatch executes exactly once and indexes its batch.

OSS-standalone CI will run the full unittests/ suite for final confirmation.

Labels

  • performance, bugfix, settings_changes (new CELERY_TASK_CONTEXT_MANAGERS in settings.dist.py)

Celery workers serve no HTTP request, so AsyncSearchContextMiddleware never
opens a watson search_context. Without an active context, django-watson's
post_save receiver indexes every saved object synchronously (one
watson_searchentry DELETE+INSERT per object), so a bulk import running in a
worker re-indexes findings one at a time — thousands of index writes.

Add watson_search_context_for_task, a context manager wired via
CELERY_TASK_CONTEXT_MANAGERS, that opens a search_context around each task so
those saves accumulate and drain in WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE-sized
async batches on exit, mirroring the request path.

Also gate the intermediate size-flush hook on the shared global manager
instance. The terminal index task (update_watson_search_index_for_model) builds
its own local SearchContextManager to index one already-bounded batch; the
class-level add_to_context monkeypatch would otherwise re-trigger the flush and
re-dispatch that task for the same batch — infinite recursion under eager
celery, an infinite re-dispatch loop (nothing ever indexed) on a real worker.
@github-actions github-actions Bot added the settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR label Jul 7, 2026
@valentijnscholten valentijnscholten added this to the 3.2.0 milestone Jul 7, 2026
The new CELERY_TASK_CONTEXT_MANAGERS default (watson_search_context_for_task)
makes finding-saving tasks dispatch one batched watson index task, so the
product-grading import/reimport perf cases gain +1 async task and shed a couple
of per-finding index queries. Counts taken from the CI auto-update
(scripts/update_performance_test_counts.py) for the standalone OSS environment.
@valentijnscholten valentijnscholten marked this pull request as ready for review July 8, 2026 07:02
@valentijnscholten valentijnscholten changed the title fix(watson): batch full-text index updates on Celery worker tasks fix(watson): fix + batch full-text index updates on Celery worker tasks Jul 8, 2026
Patch add_to_context on the module-global search_context_manager instance
instead of the SearchContextManager class. Ad-hoc contexts (e.g. the one
update_watson_search_index_for_model builds to index its own batch) keep the
stock method, so the identity guard inside the wrapper is no longer needed --
the re-dispatch loop becomes unrepresentable instead of guarded against.
watson's post_save receivers and the request/task paths all go through the
singleton, so coverage is unchanged.

Also adds the ad-hoc-context regression test (mirrors the guard test shipped
to bugfix in DefectDojo#15187).
@Maffooch Maffooch requested a review from blakeaowens July 9, 2026 01:35

@mtesauro mtesauro 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.

Approved

@mtesauro mtesauro merged commit 7857ff8 into DefectDojo:dev Jul 9, 2026
147 of 148 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR unittests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants