Skip to content

fix(watson): only intermediate-flush the global search context singleton#15187

Merged
Maffooch merged 1 commit into
DefectDojo:bugfixfrom
valentijnscholten:fix/watson-adhoc-context-no-intermediate-flush
Jul 9, 2026
Merged

fix(watson): only intermediate-flush the global search context singleton#15187
Maffooch merged 1 commit into
DefectDojo:bugfixfrom
valentijnscholten:fix/watson-adhoc-context-no-intermediate-flush

Conversation

@valentijnscholten

@valentijnscholten valentijnscholten commented Jul 8, 2026

Copy link
Copy Markdown
Member

Summary

Minimal extraction of the singleton guard from #15165 (credit @dogboat), targeted at bugfix because the affected code shipped there when the branch was cut from dev (the bug landed in #14881 in 3.1.0).

The bug

install_intermediate_flush_hook() patches SearchContextManager.add_to_context at class level, so it also fires inside update_watson_search_index_for_model()'s own ad-hoc SearchContextManager. A reindex task processing a full WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE (default 1000) batch hits the flush threshold from within the worker and re-dispatches a clone of itself, then discards those pks unindexed. Observed live on a dev stack: the worker republishes the same 1000-pk batches forever — queue length stays ~0 (consumed as fast as republished), worker pegged, nothing indexed. Flushing the broker doesn't help; in-flight tasks reseed the loop on warm shutdown.

Note the trigger is any request that saves ≥1000 watson-registered objects individually on the request thread (bulk edits, mass status changes) so the middleware drains a full-batch task. Plain imports on current code do not produce such a task (findings are bulk-created signal-less in-request and saved per-instance in worker post-processing without an active context) — but the loop detonates deterministically the moment any full-batch task is enqueued.

The fix

Guard the hook so only the global watson singleton — the request-path context managed by AsyncSearchContextMiddleware — intermediate-flushes:

if self is not search_context_manager:
    return

Ad-hoc context managers index their own batch synchronously on end(), as stock watson does. No behavior change on the request path; no call-site changes. Further improvements will follow on dev in #15172.

Verification (local, JFrog Xray Unified file with 1373 findings)

Code Trigger Result
unguarded (3.1.0 / OSS stack) direct dispatch of one 1000-pk reindex task task count grows without bound (10 → 50 → 200+ in ~3 min) until worker killed
guarded same direct dispatch exactly 1 execution (6.9s), batch indexed, silence
guarded foreground API import 7 watson tasks, all 1373 findings indexed, no loop
guarded background (async importer) import 3 watson tasks, all 1373 findings indexed, no loop

Tests

Adds test_ad_hoc_context_manager_does_not_drain to the existing unittests/test_watson_intermediate_flush.py. Full module passes locally (5/5).

The intermediate-flush hook patches SearchContextManager.add_to_context at
class level, so it also fires inside update_watson_search_index_for_model's
own ad-hoc context. A task processing a full
WATSON_ASYNC_INDEX_UPDATE_BATCH_SIZE batch re-dispatches a clone of itself,
discards those pks unindexed, and loops forever: queue length stays ~0 while
the worker is pegged republishing the same batches, and nothing gets indexed.

Guard the hook so only the global singleton (the request-path context managed
by AsyncSearchContextMiddleware) intermediate-flushes. Ad-hoc context managers
index their own batch on end(), as stock watson does.

Extracted from the hardening half of DefectDojo#15165 for the bugfix branch.

Co-authored-by: dogboat <dogboat@users.noreply.github.com>
valentijnscholten added a commit to valentijnscholten/django-DefectDojo that referenced this pull request 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).
@valentijnscholten valentijnscholten added this to the 3.1.100 milestone Jul 8, 2026
@Maffooch Maffooch requested review from blakeaowens and dogboat July 9, 2026 04:39
mtesauro pushed a commit that referenced this pull request Jul 9, 2026
…ks (#15172)

* fix(watson): batch full-text index updates on Celery worker tasks

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.

* chore(perf): update importer perf counts for batched watson index task

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.

* refactor(watson): bind intermediate-flush hook to the singleton instance

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 #15187).
@Maffooch Maffooch merged commit fa72db5 into DefectDojo:bugfix Jul 9, 2026
147 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants