Skip to content

fix(search): un-gate the staged chunk recreate from the removed recreateIndex flag - #30364

Merged
mohityadav766 merged 4 commits into
mainfrom
fix/staged-chunk-recreate-dead-guard
Aug 3, 2026
Merged

fix(search): un-gate the staged chunk recreate from the removed recreateIndex flag#30364
mohityadav766 merged 4 commits into
mainfrom
fix/staged-chunk-recreate-dead-guard

Conversation

@tomasmontielp

@tomasmontielp tomasmontielp commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #30363

The staged, generation-based chunk-index recreate (#30068) never fired in any app-driven reindex: it gated on Boolean.TRUE.equals(getJobData().getRecreateIndex()), but SearchIndexAppConfigSanitizer (#28402, "always recreate on reindex; drop the recreateIndex flag") strips that removed option from every persisted config before EventPublisherJob is built, so the flag is always null and the guard silently returned — before either the staging call or the "Partial recreate" log. Verified on a live deployment: a full Entities=All reindex with embeddings enabled promoted every per-entity *_rebuild_* index and re-embedded chunks in place, but created no _g<ts> generation, no alias, and logged none of the chunk-staging lines. Net effect: the orphaned-chunk sweep the feature exists for never runs, and the read name never migrates to the alias layout, so resolveLiveChunkTarget's alias probe hits an expected 404 that executeGenericRequest logs as an ERROR with a full stack trace once per process on every instance.

Two changes:

  • RecreateWithEmbeddings: the staging decision is now shouldStageChunkRecreate(jobData, entities) — job-driven and covering every vector-indexable type — with no reference to the removed flag (post-refactor(search): always recreate on reindex; drop the recreateIndex … #28402 every app reindex is a recreate, so job-driven implies recreate; the jobless ops-CLI createIndexes path stays excluded via the jobData null-check, and partial runs keep the existing "Partial recreate" log and never stage). Staging cost is unchanged from the original design: unchanged entities reuse their stored vectors across generations, so a full recreate does not re-embed the catalog.
  • OpenSearchVectorService.resolveLiveChunkTarget: probe the read alias with the quiet boolean existsAlias before fetching its target, instead of a throwing GET /_alias/{base} — on the pre-promotion physical-index layout the alias legitimately does not exist, and the generic-request path logged that expected 404 at ERROR. Resolution semantics are unchanged (alias target → legacy physical index → null on fresh install).

Regression tests pin the trap: shouldStageChunkRecreate must return true for a jobData deserialized from a sanitizer-shaped config (recreateIndex absent → null), false without jobData, false for partial coverage.

Type of change: Bug fix

🤖 Generated with Claude Code

Greptile Summary

This PR restores staged chunk-index recreation for full application-driven reindexes. The main changes are:

  • Replaces the removed recreateIndex gate with job and entity-coverage checks.
  • Uses strict live-target resolution before orphan cleanup and alias promotion.
  • Keeps lenient resolution for the non-destructive create-if-absent path.
  • Adds tests for sanitized job data, partial runs, jobless runs, and failed alias probes.

Confidence Score: 5/5

This looks safe to merge.

  • Destructive paths now stop when the live target probe fails.
  • The lenient resolver is limited to index creation.
  • No blocking issues remain in the changed code.

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/search/RecreateWithEmbeddings.java Stages chunk recreation for full job-driven vector reindexes without consulting the removed configuration flag.
openmetadata-service/src/main/java/org/openmetadata/service/search/vector/OpenSearchVectorService.java Makes destructive staging and promotion abort when the live chunk target cannot be resolved.
openmetadata-service/src/test/java/org/openmetadata/service/search/RecreateWithEmbeddingsTest.java Tests staging decisions for sanitized, jobless, and partial reindex inputs.
openmetadata-service/src/test/java/org/openmetadata/service/search/vector/OpenSearchVectorServiceChunkStagingTest.java Tests that an indeterminate alias probe aborts staging before index mutation.

Reviews (4): Last reviewed commit: "Merge branch 'main' into fix/staged-chun..." | Re-trigger Greptile

Context used:

  • Context used - CLAUDE.md (source)
  • Context used - AGENTS.md (source)

…ateIndex flag

The staged chunk-index recreate (#30068) required
jobData.recreateIndex == true, but SearchIndexAppConfigSanitizer (#28402)
strips that removed option from every persisted config before jobData is
built, so the guard could never pass: no staged generation, no alias
promotion, no orphaned-chunk sweep — chunks silently fell back to the
in-place rebuild while every per-entity index kept recreating. A
job-driven run covering every vector-indexable type now stages the chunk
recreate unconditionally (reindexes are always recreate-mode
post-#28402); the jobless ops-CLI createIndexes path stays excluded via
the jobData null-check.

Also probe the read alias with the quiet boolean existsAlias instead of
a throwing GET /_alias/{base}: on the pre-promotion physical-index
layout the alias legitimately does not exist, and executeGenericRequest
logged that expected 404 as an ERROR with a full stack trace once per
process on every instance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added the safe to test Add this label to run secure Github workflows on PRs label Jul 22, 2026
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit d2295b7f3b8ded371325633458a516faad041f4e in Playwright run 30639850597, attempt 1.

✅ 1024 passed · ❌ 0 failed · 🟡 2 flaky · ⏭️ 3 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) 59m 55s

⏱️ Max setup 2m 53s · max shard execution 15m 59s · max shard-job elapsed before upload 20m 54s · reporting 7s

🌐 188.14 requests/attempt · 2.22 app boots/UI scenario · 20.10% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 20.1% (convergence target: at most 15%).
  • Application boot ratio was 2.22 per UI scenario (2315 boots / 1044 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
🟡 Shard chromium-01 135 0 2 0 0 0
✅ Shard chromium-02 142 0 0 3 0 0
✅ Shard chromium-03 135 0 0 0 0 0
✅ Shard chromium-04 143 0 0 0 0 0
✅ Shard chromium-05 135 0 0 0 0 0
✅ Shard chromium-06 148 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
🟡 2 flaky test(s) (passed on retry)
  • Pages/Entity.spec.tsDomain Propagation (shard chromium-01, 1 retry)
  • Pages/Entity.spec.tsDomain Propagation (shard chromium-01, 1 retry)

📦 Download artifacts

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

@tomasmontielp
tomasmontielp requested a review from a team as a code owner July 30, 2026 08:27
@mohityadav766 mohityadav766 added the To release Will cherry-pick this PR into the release branch label Aug 3, 2026
@mohityadav766
mohityadav766 merged commit 1ae5a62 into main Aug 3, 2026
170 of 175 checks passed
@mohityadav766
mohityadav766 deleted the fix/staged-chunk-recreate-dead-guard branch August 3, 2026 12:48
mohityadav766 pushed a commit that referenced this pull request Aug 3, 2026
…ateIndex flag (#30364)

* fix(search): un-gate the staged chunk recreate from the removed recreateIndex flag

The staged chunk-index recreate (#30068) required
jobData.recreateIndex == true, but SearchIndexAppConfigSanitizer (#28402)
strips that removed option from every persisted config before jobData is
built, so the guard could never pass: no staged generation, no alias
promotion, no orphaned-chunk sweep — chunks silently fell back to the
in-place rebuild while every per-entity index kept recreating. A
job-driven run covering every vector-indexable type now stages the chunk
recreate unconditionally (reindexes are always recreate-mode
post-#28402); the jobless ops-CLI createIndexes path stays excluded via
the jobData null-check.

Also probe the read alias with the quiet boolean existsAlias instead of
a throwing GET /_alias/{base}: on the pre-promotion physical-index
layout the alias legitimately does not exist, and executeGenericRequest
logged that expected 404 as an ERROR with a full stack trace once per
process on every instance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* greptile comments

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit 1ae5a62)
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Failed to cherry-pick changes to the 2.0 branch.
Please cherry-pick the changes manually.
You can find more details here.

@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Restores generation-based chunk recreation for full app-driven reindexes by removing the obsolete recreateIndex flag guard and updating alias probing. 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 👍 / 👎 | Gitar | Powered by Gitar — free for open source

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Failed to cherry-pick changes to the 1.13 branch.
Please cherry-pick the changes manually.
You can find more details here.

@mohityadav766

Copy link
Copy Markdown
Member

Heads-up: this change makes a full reindex hard-depend on the embedding provider being reachable, and that has been breaking the nightly Java IT suites on main and 2.0 since 2026-08-04. Filing here since the fix touches this code directly.

What happens

Un-gating the staged chunk recreate means every job-driven full recreate now reaches beginStagedChunkRecreate(), which opens with preflightEmbedding(). That rethrows, and the exception fails the entire SearchIndexApp run — so a reindex that needs no embeddings at all is blocked when the provider is unavailable:

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)

It surfaces as status='failed' within seconds with an empty failureContext, which made it genuinely hard to trace back here.

Why it affects more than CI

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

So on any deployment where a region resolves (anything on AWS) but bedrock:InvokeModel was never granted, the client constructs fine, initializeVectorSearchService reports success, and the first full reindex dies. Note the asymmetry that hides it: live indexing logs embedding errors and carries on, so the deployment looks healthy right up until someone reindexes.

Two configurations behave differently, worth being explicit about:

config outcome
no AWS region resolvable constructor throws → caught in initializeVectorSearchService → vector service off → reindex fine
region resolves, no bedrock:InvokeModel constructor succeeds → pre-flight 403s → every full reindex fails

OSS is unaffected — semanticSearchEnabled defaults to false, so getInstance() is null and recreateChunkIndexIfFullRun returns early.

Evidence it's this commit

run image built outcome
2026-08-03 00:45Z 2.0-202608030000 00:00Z green
2026-08-03 13:08Z 2.0-202608031227 12:27Z ui-it + search-it green
2026-08-04 00:46Z 2.0-202608040000 00:00Z reindex fails, run cancelled

67f8caa49b landed at 12:49Z on Aug 3 — 22 minutes after the last image whose reindex still worked, and before the first that broke. 1.13 doesn't carry the commit and its reindex is healthy, which matches the branch split exactly.

Proposed fix

#31131 (main) / #31132 (2.0): treat an unavailable provider as "do not stage" rather than "fail" — return null, the outcome the caller already handles for a partial recreate, so existing chunks stay live and are swept by the next recreate with a working provider. markEntityTypeReindexed already ignores marks from a run without staging, so it reuses a supported state.

The pre-flight's intent is kept — 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 (indeterminate live-target probe, failed index create) still throw, and beginStagedChunkRecreate_abortsWhenTheLiveTargetProbeIsIndeterminate passes unchanged.

Separately, open-metadata/openmetadata-nightly#285 points the Java IT cluster at DJL in-process embeddings so CI stops depending on Bedrock at all.

Since 67f8caa49b is on the 2.0 release branch, this ships unless addressed. Happy to adjust the approach if you'd rather gate it differently — e.g. keeping the throw but only when the provider was explicitly configured.

mohityadav766 added a commit that referenced this pull request Aug 6, 2026
…reachable (#31132)

#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.
c3us-dev pushed a commit to c3us-dev/OpenMetadata that referenced this pull request Aug 7, 2026
…reachable (open-metadata#31131)

open-metadata#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Staged chunk-index recreate never fires: gated on the removed recreateIndex flag

2 participants