Skip to content

fix(search): don't fail the reindex when the embedding provider is unreachable - #31131

Merged
mohityadav766 merged 4 commits into
mainfrom
fix/reindex-embedding-optional-main
Aug 7, 2026
Merged

fix(search): don't fail the reindex when the embedding provider is unreachable#31131
mohityadav766 merged 4 commits into
mainfrom
fix/reindex-embedding-optional-main

Conversation

@mohityadav766

Copy link
Copy Markdown
Member

Follows up #30364, which is on main and the 2.0 release branch.

What happens today

#30364 un-gated the staged chunk recreate, so every job-driven full recreate now calls beginStagedChunkRecreate(), which opens with a pre-flight embed. That pre-flight rethrows, and the exception fails the entire SearchIndexApp run — so a reindex that needs no embeddings at all is blocked by an optional AI provider being unreachable.

Why this is not a narrow case

Collate ships naturalLanguageSearch.semanticSearchEnabled=true by default, while llmConfiguration.embeddings.provider defaults to bedrock. And BedrockEmbeddingClient's constructor never calls AWS — it validates model id, dimension and region, then builds the SDK client.

So on any deployment where a region resolves (anything running on AWS) but bedrock:InvokeModel was never granted:

  1. the client constructs happily,
  2. initializeVectorSearchService reports success,
  3. and the first full reindex dies:
User: arn:aws:sts::...:assumed-role/... is not authorized to perform:
bedrock:InvokeModel on resource: .../amazon.titan-embed-text-v2:0
(Service: BedrockRuntime, Status Code: 403)

surfacing as status='failed' within seconds with an empty failureContext.

The misconfiguration is otherwise invisible. Live indexing logs embedding errors and carries on, so the deployment looks healthy right up until someone reindexes. OSS is unaffected — semanticSearchEnabled defaults to false there, so the vector service never initialises and the gate returns early.

This is what has been failing the nightly Java IT reindex suites on main and 2.0 since 2026-08-04.

The change

Treat an unavailable provider as "do not stage" rather than "fail": return null, which the caller already handles as the partial-recreate outcome — existing chunks stay live and get swept by the next recreate that runs with a working provider. markEntityTypeReindexed already ignores marks from a run without staging (the "unbound chunk-type mark" branch), so this reuses a supported state rather than inventing one.

The pre-flight's intent is preserved: it still refuses to stage a generation it could never finish. It just no longer takes the entity reindex down with it.

Genuine staging failures still throw — an indeterminate live-target probe, a failed index create — because those mean continuing could destroy live chunks. The existing beginStagedChunkRecreate_abortsWhenTheLiveTargetProbeIsIndeterminate test still passes unchanged.

Test plan

  • OpenSearchVectorServiceChunkStagingTest — 8/8 pass, including a new case asserting an unavailable provider returns null and touches nothing in the cluster (verify(client, never()).indices() / .generic()).
  • RecreateWithEmbeddingsTest — 7/7 pass.
  • Nightly Java IT once merged.

Unit tests were executed on main; the 2.0 change is character-identical (verified by diffing the two patches) but its module could not be built locally offline.

Related

🤖 Generated with Claude Code

…reachable

#30364 un-gated the staged chunk recreate, so every job-driven full recreate now
calls beginStagedChunkRecreate(), which opens with a pre-flight embed. The
pre-flight rethrows, and that exception fails the whole SearchIndexApp run — so
a reindex that needs no embeddings at all is now blocked by an optional AI
provider being unreachable.

That is not a narrow case. Collate ships semanticSearchEnabled=true by default
while llmConfiguration.embeddings.provider defaults to bedrock, and
BedrockEmbeddingClient's constructor never calls AWS — it validates the model id,
dimension and region, then builds the SDK client. So on any deployment where a
region resolves (anything on AWS) but bedrock:InvokeModel was never granted, the
client constructs happily, initializeVectorSearchService reports success, and the
first full reindex dies:

  User: arn:aws:sts::...:assumed-role/... is not authorized to perform:
  bedrock:InvokeModel on resource: .../amazon.titan-embed-text-v2:0
  (Status Code: 403)

surfacing as status='failed' within seconds with an empty failureContext. The
misconfiguration is otherwise invisible: live indexing logs embedding errors and
carries on, so the deployment looks healthy right up until someone reindexes.
This is what has been failing the nightly Java IT suites on main and 2.0.

Treat an unavailable provider as "do not stage" rather than "fail": return null,
which the caller already handles as the partial-recreate outcome — existing
chunks stay live and are swept by the next recreate that runs with a working
provider. markEntityTypeReindexed already ignores marks from a run without
staging, so this reuses a supported state rather than inventing one.

Genuine staging failures — an indeterminate live-target probe, a failed index
create — still throw, because those mean continuing could destroy live chunks.
@mohityadav766
mohityadav766 requested a review from a team as a code owner August 6, 2026 16:33
Copilot AI review requested due to automatic review settings August 6, 2026 16:33

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adjusts staged chunk recreation to skip staging (instead of failing the whole reindex) when the embedding provider is unreachable.

Changes:

  • beginStagedChunkRecreate() now returns null when embedding pre-flight fails, allowing the caller to proceed with an in-place reindex.
  • Replaces throwing pre-flight method with isEmbeddingAvailable() that logs and returns a boolean.
  • Adds a regression test covering the “embedding unavailable ⇒ do not touch cluster and return null” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
openmetadata-service/src/main/java/org/openmetadata/service/search/vector/OpenSearchVectorService.java Implements “skip staging when embedding provider is unavailable” via a boolean pre-flight and a nullable return.
openmetadata-service/src/test/java/org/openmetadata/service/search/vector/OpenSearchVectorServiceChunkStagingTest.java Adds test asserting beginStagedChunkRecreate() returns null and makes no OpenSearch calls when embeddings fail.

Comment on lines 461 to +468
public String beginStagedChunkRecreate() {
preflightEmbedding();
if (!isEmbeddingAvailable()) {
LOG.warn(
"Embedding pre-flight failed — skipping the staged chunk recreate. The entity reindex "
+ "continues and existing chunks stay live; orphaned chunks, if any, are swept by the "
+ "next recreate that runs with a working embedding provider.");
return null;
}
Comment on lines +514 to 522
private boolean isEmbeddingAvailable() {
try {
embeddingClient.embedQuery("chunk index recreate pre-flight");
return true;
} catch (Exception e) {
throw new RuntimeException(
"Refusing to start a staged chunk-index recreate: embedding client pre-flight failed", e);
LOG.warn("Embedding client pre-flight failed: {}", e.getMessage(), e);
return false;
}
}
@mohityadav766
mohityadav766 enabled auto-merge August 7, 2026 06:01
Copilot AI review requested due to automatic review settings August 7, 2026 10:02
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Aug 7, 2026

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit b34b501904e7960a74e2f500e5b2380d81b28439 in Playwright run 31187328307, attempt 1.

✅ 1026 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 55m 36s

⏱️ Max setup 3m 2s · max shard execution 19m 17s · max shard-job elapsed before upload 24m 13s · reporting 6s

🌐 189.20 requests/attempt · 2.21 app boots/UI scenario · 26.88% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 26.88% (convergence target: at most 15%).
  • Application boot ratio was 2.21 per UI scenario (2308 boots / 1042 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 158 0 0 0 0 0
✅ Shard chromium-02 161 0 0 0 0 0
✅ Shard chromium-03 170 0 0 0 0 0
✅ Shard chromium-04 179 0 0 0 0 0
✅ Shard chromium-05 172 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard import-export-01 7 0 0 0 0 0
✅ Shard ingestion-01 2 0 0 0 0 0
✅ Shard reindex-01 28 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@mohityadav766
mohityadav766 removed this pull request from the merge queue due to a manual request Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🚦 Removed from the merge queue — manual (2026-08-07T14:20:48Z)

The entry left the queue before it was built, so no checks ran against it.

Copilot AI review requested due to automatic review settings August 7, 2026 14:23
@mohityadav766
mohityadav766 enabled auto-merge August 7, 2026 14:23

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

openmetadata-service/src/main/java/org/openmetadata/service/search/vector/OpenSearchVectorService.java:521

  • isEmbeddingAvailable() currently swallows all exceptions from embedQuery() and turns them into "not staged". If the thread is interrupted (e.g., shutdown/cancellation while waiting for a permit), we should not continue the reindex; treat that as a genuine failure and rethrow, and narrow the catch to RuntimeException (the only type embedQuery can throw).
      LOG.warn("Embedding client pre-flight failed: {}", e.getMessage(), e);
      return false;
    }

openmetadata-service/src/main/java/org/openmetadata/service/search/vector/OpenSearchVectorService.java:517

  • isEmbeddingAvailable() always performs an embedQuery() call, which can block on provider timeouts even when the embedding client circuit breaker is already open. Consider short-circuiting via embeddingClient.isAvailable() first to skip staging without making a provider call during an outage (you’ll need to update unit tests to stub isAvailable() where appropriate).
  private boolean isEmbeddingAvailable() {
    try {
      embeddingClient.embedQuery("chunk index recreate pre-flight");
      return true;

@mohityadav766
mohityadav766 added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit bc88ca3 Aug 7, 2026
111 of 114 checks passed
@mohityadav766
mohityadav766 deleted the fix/reindex-embedding-optional-main branch August 7, 2026 19:28
@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Makes embedding provider failures non-fatal during search reindexing by treating unreachable providers as a skip condition rather than failing the run. No issues found.

Options

Display: compact → Showing less information.

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

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.

3 participants