fix: bound the synchronous Meilisearch wait when indexing library blocks - #39072
fix: bound the synchronous Meilisearch wait when indexing library blocks#39072AhtishamShahid wants to merge 2 commits into
Conversation
Creating or editing a v2 library block indexes it synchronously inside the request,
via `upsert_library_block_index_doc.apply()`. That call ends in
`_wait_for_meili_task()`, which polls until Meilisearch reports the task finished
with no timeout, using a client built with no socket timeout either. So the HTTP
response is held open for as long as the search backend takes.
The write itself has already committed by then: `LibraryBlocksView` is
`non_atomic_requests` and the content-library write happens in its own
`transaction.atomic()`, and the events that trigger indexing only fire after that
commit. So a slow index cannot roll the write back - it only delays the response.
When a gateway in front of Studio times out first, the user sees a 5xx for a
request that actually succeeded, and retrying creates another orphaned block.
Bound the wait instead:
- `_wait_for_meili_task()` takes an optional `timeout` and returns whether the
task completed. On timeout it logs and returns rather than polling forever. The
backoff is also clamped so a 2s sleep cannot overshoot a shorter deadline.
- The Meilisearch client is constructed with a socket timeout, so a backend that
accepts the connection and then stops responding can no longer pin a worker.
- `_update_index_docs()` and `api.upsert_library_block_index_doc()` thread the
timeout through, and the two library-block handlers pass
`SYNC_INDEX_WAIT_TIMEOUT`.
Giving up on the wait does not lose the update. Once `update_documents()` returns
a task uid, Meilisearch applies the documents regardless; waiting only told us
when. The default stays `None` (wait indefinitely), so reindex and the management
commands are unchanged - only the two interactive paths are bounded.
Measured against a Meilisearch proxied to defer applying writes:
healthy, timeout=3 -> 0.10s
slow 20s, timeout=3 -> 3.01s
slow 20s, timeout=None -> 21.27s (previous behaviour)
documents present in all three cases
End to end through Studio with indexing deferred 100s, this changes
`POST /api/libraries/v2/{lib}/blocks/` from no response at 60s to a 200 in ~3.2s.
Note this bounds how long the *request* waits, not how long indexing takes. The
Authoring MFE lists components from the search index, so on a slow backend a new
component can take a few seconds to appear in the listing. That is a better
failure mode than a gateway error plus an orphaned block, but fully closing the
gap needs the frontend to render the new component from the create response.
Refs: openedx#38993
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @AhtishamShahid! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Wrap the call to the module-private helper in a single thin function with one waiver, instead of repeating the disable at five call sites. Deliberately a function rather than a module-level alias: `api` is imported inside a try/except in this module because the import raises in the LMS, so resolving `api._wait_for_meili_task` at import time would break collection on LMS test runs even though these tests are CMS-only. Verified LMS collection still succeeds.
Is this happening on Verawood / master ? Because what you're describing sounds like the behavior we had on Ulmo, but in the more recent versions, the indexing tasks happen in a celery worker and the Note that you will not be able to see this behavior correctly on a stock Tutor devstack, as tutor doesn't enable celery workers to run on separate processes in dev mode. However, you can turn that on manually to see it. There is also a very relevant and detailed discussion about indexing slowness taking place here: openedx/openedx-platform#38993 (Edit: oh, I see you linked to that already.) |
|
See also #39042 which I just discovered in the PR queue here. |
4 participants
Problem
When a v2 library block is created or edited, it is indexed in Meilisearch synchronously as part of the same request.
The code waits for Meilisearch to finish indexing using
_wait_for_meili_task(). Previously, there was no timeout, and the Meilisearch client also had no socket timeout.This means that if Meilisearch becomes slow or gets stuck, the HTTP request can remain open indefinitely.
There is another important issue: the database write has already been committed before indexing starts because the indexing events run after the database transaction commits.
So, a slow Meilisearch operation cannot undo the block creation/update. It only delays the HTTP response.
For example:
Solution
We now put a limit on how long the request waits for Meilisearch.
_wait_for_meili_task()now accepts an optionaltimeoutand returns whether the Meilisearch task finished within that time.The Meilisearch client also has a socket timeout.
For the two library-block API handlers, we set:
SYNC_INDEX_WAIT_TIMEOUT = 3sSo the API request will wait for Meilisearch for up to 3 seconds, instead of waiting forever.
Importantly, giving up on waiting does not mean the update is lost.
Once
update_documents()successfully returns a Meilisearch task UID, Meilisearch will still process the documents. The timeout only means that we stop waiting for it to finish.The default timeout remains
None, so existing callers such asreindex_studioand the management commands continue to behave exactly as before.Results
With indexing artificially delayed, the results were:
The documents were present in Meilisearch in all three cases, confirming that timing out the wait does not lose the update.
End-to-end through Studio, with indexing artificially delayed by 100 seconds:
POST /api/libraries/v2/{lib}/blocks/produced no response even after 60 seconds.We also added 5 new tests. Both
test_api.pyandtest_handlers.pypass, with 44 tests passing in total.One remaining limitation
This change limits how long the API request waits, but it does not make Meilisearch indexing itself faster.
The Authoring MFE gets its component list from the search index. Therefore, if Meilisearch is slow, a newly created component may still take a few seconds to appear in the UI.
This is still much better than having the gateway return an error for a successful request and potentially leaving an orphaned block behind.
Fully eliminating the delay between creating a component and seeing it in the UI would require a frontend change.
Reference: #38993