Skip to content

Fixes #35024: retry search update-by-query version conflicts against a refreshed index - #35027

Merged
manerow merged 3 commits into
mainfrom
fix/search-update-by-query-retry
Oct 15, 2026
Merged

manerow merged 3 commits into
mainfrom
fix/search-update-by-query-retry

Conversation

@manerow

@manerow manerow commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #35024

Problem

Several changes update many search documents with one update-by-query: renaming a glossary term, tag or classification, moving a data product to another domain, changing a parent's fully qualified name, updating lineage or relationships, soft deleting or restoring a parent's children. If another write touches one of those documents during the scan, the engine reports a version conflict for it.

  • On OpenSearch, and for most helpers on Elasticsearch, that document is skipped and keeps its old value until the next reindex, usually without a log line.
  • The glossary term, classification and data product reference updates on Elasticsearch, and the soft delete and restore of children on both engines, used the default abort, so the first conflict stopped the update and every document after it kept its old value too.

The database is right in every case; only search is wrong.

What this PR changes

UpdateByQueryReconciler, generalised from ColumnLineageReconciler (which did this for column lineage only), runs an update-by-query and reads its outcome. When there are version conflicts and the script is safe to run twice, it refreshes the target indices so the next attempt sees the latest documents, and tries again, at most 3 attempts in total. A script is not safe to replay when a second run would change an updated document again: renaming the prefix a to a.b is that case, because a.b still starts with a. Those run once.

UpdateByQueryOutcome reports what is left after the last attempt: a warning with the operation, the indices, the count and the attempts made, or an error for real failures. When the caller knows the document ids, as the data product domain update does, they go to the search retry queue.

The 12 synchronous update-by-query helpers of each engine now run through the reconciler, and all of them use proceed. ColumnLineageReconciler keeps its behaviour and delegates to the shared code. The rule is recorded in docs/decisions/.

What users will notice

Renames, domain moves, propagation and soft deletes that race with another edit reach every document they should. Anything still left behind is in the server log.

How it was tested

Unit tests for the reconciler (retries until clean, stops after 3 attempts, no retry when a replay is unsafe, refresh between attempts, leftovers reported) and for both engine managers (helpers retry on conflicts and use proceed, the soft delete and restore of children included). The soft delete and restore suites pass on both engines (234 tests each).

Integration suites for glossary terms, glossaries, classifications, tags, domains, data products, data product domain migration and lineage: 1,607 tests pass on OpenSearch. On Elasticsearch, one certification rename test failed once; it uses a background update this PR does not change and passed in two reruns.

Not in this PR

Child updates sent to the cluster as background tasks (wait_for_completion=false): the server does not wait for those, so the request has nothing to retry.

@manerow
manerow requested a review from a team as a code owner October 9, 2026 12:54
@manerow manerow added safe to test Add this label to run secure Github workflows on PRs backend labels Oct 9, 2026
@manerow manerow self-assigned this Oct 9, 2026
@gitar-bot

gitar-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 2 closed / 2 findings

🔴 High risk · Changing retries for bulk search-index updates, including soft deletes and renames, could replay scripts incorrectly or leave many entity documents inconsistent.

Adds UpdateByQueryReconciler so search update-by-query operations retry version conflicts against a refreshed index, with leftovers reported as warnings or errors. Two findings were resolved: the conflict policy is now documented in docs/decisions/ rather than only in the PR description, and the leftover-conflict warning now reports the attempts actually made instead of always claiming 3. No open issues remain.

✅ 2 closed
✅ Quality: Update-by-query conflict policy is stated only in the PR description

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/UpdateByQueryReconciler.java:8-22 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/UpdateByQueryReconciler.java:63-70 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchEntityManager.java:1029 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchEntityManager.java:1083 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/ElasticSearchEntityManager.java:1137 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/OpenSearchEntityManager.java:1039-1048
This PR turns what used to be a column-lineage detail into a rule for every synchronous update-by-query helper on both engines. The rule lives only in the description: "The 11 synchronous update-by-query helpers of each engine now run through the reconciler. The three Elasticsearch helpers that used abort now use proceed", with retries "at most 3 attempts in total" and scripts that are not replay-safe running once. The repository squash-merges, so a later helper written with the default abort, or one that calls client.updateByQuery directly, would quietly undo this. Background wait_for_completion=false updates are also left out on purpose.

The tree has the reconciler's Javadoc and the idempotency note on updateChildren. No record in docs/decisions/ says the following, so one should state it: "Every synchronous update-by-query in ElasticSearchEntityManager/OpenSearchEntityManager runs with conflicts=proceed through UpdateByQueryReconciler, which refreshes and retries version conflicts up to 3 attempts in total. A script that is not replay-safe (for example a prefix rename whose new prefix nests under the old one) runs once. Leftover conflicts are logged, and are re-queued by id when the caller knows the ids. Background (wait_for_completion=false) child updates are excluded."

✅ Bug: Leftover-conflict warning always claims 3 attempts were made

📄 openmetadata-service/src/main/java/org/openmetadata/service/search/UpdateByQueryOutcome.java:30-37 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/UpdateByQueryOutcome.java:47-50 📄 openmetadata-service/src/main/java/org/openmetadata/service/search/UpdateByQueryReconciler.java:44-58
report() and requeueIfConflicted() always print UpdateByQueryReconciler.MAX_ATTEMPTS as the number of attempts. In many cases only one attempt ran. That happens when the script is not replay-safe (for example a glossary rename a -> a.b, or updateByFqnPrefix when the new FQN contains the old one). It also happens when a shard failure stops the loop after the first run. In those cases the WARN line and the retry-queue failure reason both say "after 3 attempts", which points operators the wrong way: it suggests retries kept failing, when no retry was tried. The fix is to carry the real attempt count from reconcile into the outcome, for example as an attempts field set through a withAttempts method, and log that number.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Options

Display: compact → Showing compact verification results.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

@manerow
manerow added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit c4f204a Oct 15, 2026
77 of 78 checks passed
@manerow
manerow deleted the fix/search-update-by-query-retry branch October 15, 2026 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Search updates across many entries silently skip entries that are being written at the same time

2 participants