From d8f831ad40b1195b81e052417c79f2f556b9f715 Mon Sep 17 00:00:00 2001 From: Tobias Macey Date: Thu, 22 Oct 2026 19:18:25 -0400 Subject: [PATCH] fix: bound how long content/search waits on Meilisearch document writes #39072 bounded the wait for inline library block upserts with SYNC_INDEX_WAIT_TIMEOUT. The other incremental writes still poll with no deadline, and the library handlers run several of them inline with .apply(): block deletes, library updates, and collection and container writes. Meilisearch applies tasks one at a time in enqueue order across every index, so a one-document write waits behind whatever is queued ahead of it, and a backed-up queue turns those Studio requests into gateway timeouts for changes that were saved. This adds MEILISEARCH_WRITE_WAIT_TIMEOUT (default None, which keeps the current behavior). _update_index_docs, _delete_documents and _delete_index_doc use it when the caller does not pass its own wait_timeout, so the library block upserts keep SYNC_INDEX_WAIT_TIMEOUT. Index creation, rebuilds and settings updates keep waiting until done. Claude-Session: https://claude.ai/code/session_01JxJWMHVXsuXaChbeEpKT8t --- cms/envs/common.py | 13 +++++ openedx/core/djangoapps/content/search/api.py | 21 ++++++-- .../content/search/tests/test_api.py | 48 +++++++++++++++++++ 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/cms/envs/common.py b/cms/envs/common.py index 87ee13f3e0a6..9746a1fc6095 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -1358,6 +1358,19 @@ def _should_send_xblock_events(settings): # See https://www.meilisearch.com/docs/learn/security/tenant_tokens MEILISEARCH_INDEX_PREFIX = "" MEILISEARCH_API_KEY = "devkey" +# .. setting_name: MEILISEARCH_WRITE_WAIT_TIMEOUT +# .. setting_default: None +# .. setting_description: Seconds to wait for Meilisearch to apply each incremental document write (the writes made +# when course or library content changes, not index rebuilds) before giving up on waiting. None waits until +# Meilisearch finishes. Meilisearch applies tasks one at a time in the order they were enqueued, so a write made +# during a large reindex can wait minutes behind it, and the library handlers make these writes inline in the Studio +# request. Inline library block upserts already use SYNC_INDEX_WAIT_TIMEOUT; this covers the rest (block deletes, +# library, collection and container writes). The limit is per write, and one library change can make several inline +# writes (one per affected component, unit, subsection or section), so keep the timeout times that count below your +# gateway's upstream timeout. A write that is not waited for is still applied later, but until then the Authoring +# MFE's refetch of the library listing will not show it, and if it fails the failure is not raised (so a Celery task +# making it is not retried). +MEILISEARCH_WRITE_WAIT_TIMEOUT = None # .. setting_name: LIBRARY_ENABLED_BLOCKS # .. setting_default: ['problem', 'video', 'html', 'drag-and-drop-v2'] diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index e1502a33d535..443df158bcec 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -228,6 +228,14 @@ def _wait_for_meili_task(info: TaskInfo, timeout: float | None = None) -> bool: return True +def _write_wait_timeout() -> float | None: + """ + How long the incremental document write helpers wait for Meilisearch to apply a write. See + MEILISEARCH_WRITE_WAIT_TIMEOUT. + """ + return getattr(settings, "MEILISEARCH_WRITE_WAIT_TIMEOUT", None) + + def _index_exists(index_name: str) -> bool: """ Check if an index exists @@ -375,8 +383,8 @@ def _update_index_docs(index_name: str, docs, wait_timeout: float | None = None) If there is a rebuild of that index in progress, the document will also be added to the new index. - ``wait_timeout`` bounds how long we wait for Meilisearch to finish applying the update. The - documents are applied either way; see ``_wait_for_meili_task``. + ``wait_timeout`` bounds how long we wait for Meilisearch to finish applying the update, and defaults to + MEILISEARCH_WRITE_WAIT_TIMEOUT. The documents are applied either way; see ``_wait_for_meili_task``. """ if not docs: return @@ -387,7 +395,10 @@ def _update_index_docs(index_name: str, docs, wait_timeout: float | None = None) if current_rebuild_index_name: # If there is a rebuild in progress, the document will also be added to the new index. client.index(current_rebuild_index_name).update_documents(docs) - _wait_for_meili_task(client.index(index_name).update_documents(docs), timeout=wait_timeout) + _wait_for_meili_task( + client.index(index_name).update_documents(docs), + timeout=wait_timeout if wait_timeout is not None else _write_wait_timeout(), + ) def only_if_meilisearch_enabled(f): @@ -957,7 +968,7 @@ def _delete_documents(index_name: str, filter_query: str) -> None: if current_rebuild_index_name: # If there is a rebuild in progress, the document will also be removed from the new index. client.index(current_rebuild_index_name).delete_documents(filter=filter_query) - _wait_for_meili_task(client.index(index_name).delete_documents(filter=filter_query)) + _wait_for_meili_task(client.index(index_name).delete_documents(filter=filter_query), timeout=_write_wait_timeout()) def _delete_index_doc(index_name: str, doc_id) -> None: @@ -976,7 +987,7 @@ def _delete_index_doc(index_name: str, doc_id) -> None: # If there is a rebuild in progress, the document will also be removed from the new index. client.index(current_rebuild_index_name).delete_document(doc_id) - _wait_for_meili_task(client.index(index_name).delete_document(doc_id)) + _wait_for_meili_task(client.index(index_name).delete_document(doc_id), timeout=_write_wait_timeout()) def upsert_library_block_index_doc(usage_key: UsageKey, wait_timeout: float | None = None) -> None: diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index b814bf0f66c8..8412a1044035 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -1685,3 +1685,51 @@ def test_waits_indefinitely_by_default(self): with patch.object(api, "_MEILI_CLIENT", client): assert _wait_for_meili_task(MagicMock(task_uid=5)) is True assert client.get_task.call_count == 2 + + +@skip_unless_cms +@override_settings(MEILISEARCH_ENABLED=True) +@patch("openedx.core.djangoapps.content.search.api._wait_for_meili_task") +@patch("openedx.core.djangoapps.content.search.api.MeilisearchClient") +class TestDocumentWriteWaitTimeout(TestCase): + """ + Tests for how long the incremental document write helpers wait on Meilisearch. + """ + # pylint: disable=protected-access + + def setUp(self) -> None: + super().setUp() + api.clear_meilisearch_client() + self.addCleanup(api.clear_meilisearch_client) + + @override_settings(MEILISEARCH_WRITE_WAIT_TIMEOUT=5) + def test_document_writes_use_write_wait_timeout(self, mock_meilisearch, mock_wait) -> None: + """ + Incremental document writes and deletes wait at most MEILISEARCH_WRITE_WAIT_TIMEOUT. + """ + api._update_index_docs("idx", [{"id": "a"}]) + api._delete_index_doc("idx", "a") + api._delete_documents("idx", 'context_key = "x"') + + assert mock_wait.call_count == 3 + for wait_call in mock_wait.call_args_list: + assert wait_call.kwargs == {"timeout": 5} + + @override_settings(MEILISEARCH_WRITE_WAIT_TIMEOUT=5) + def test_explicit_wait_timeout_overrides_setting(self, mock_meilisearch, mock_wait) -> None: + """ + A caller's wait_timeout (e.g. SYNC_INDEX_WAIT_TIMEOUT for inline library block upserts) wins over the setting. + """ + api._update_index_docs("idx", [{"id": "a"}], wait_timeout=api.SYNC_INDEX_WAIT_TIMEOUT) + + assert mock_wait.call_args.kwargs == {"timeout": api.SYNC_INDEX_WAIT_TIMEOUT} + + def test_document_writes_wait_indefinitely_by_default(self, mock_meilisearch, mock_wait) -> None: + """ + Without MEILISEARCH_WRITE_WAIT_TIMEOUT, document writes wait as long as Meilisearch takes. + """ + api._update_index_docs("idx", [{"id": "a"}]) + api._delete_index_doc("idx", "a") + + for wait_call in mock_wait.call_args_list: + assert wait_call.kwargs == {"timeout": None}