Revert "fix(search): migrate stale owner aggregation field in stored searchSettings (#29691)"#29722
Closed
mohityadav766 wants to merge 1 commit into
Closed
Revert "fix(search): migrate stale owner aggregation field in stored searchSettings (#29691)"#29722mohityadav766 wants to merge 1 commit into
mohityadav766 wants to merge 1 commit into
Conversation
…searchSettings (1.13.2) (#29691)" This reverts commit 201968f. The owner-aggregation settings migration diagnosed the wrong root cause. The actual failure on the affected clusters (1.12.13) is that the reindex alias swap is rejected by OpenSearch — `must_exist` on the `remove_index` action makes the `_aliases` parser fail with `[aliases] failed to parse field [actions]` (HTTP 400), so no rebuilt index is ever promoted. That is already fixed on main by #28700; the fix simply needs backporting to 1.12.13. The searchSettings owner-aggregation scrub is unnecessary and lands in a phantom 1.13.2 migration folder, so it is reverted here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
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 |
github-actions
Bot
added
backend
safe to test
Code Review ✅ ApprovedReverts the stale owner aggregation migration as it incorrectly targeted a symptom of a parser bug already resolved in main. This cleanup removes the unnecessary migration folder and prepares the codebase for a focused backport of the alias swap fix. OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
Contributor
There was a problem hiding this comment.
Pull request overview
Reverts the previously added 1.13.2 data-only migration that retargeted the stored searchSettings owner aggregation field (owners.displayName.keyword → ownerDisplayName), along with its tests and versioned SQL markers, based on updated understanding that the underlying failure is in the OpenSearch alias swap (handled elsewhere).
Changes:
- Removes
v1132Java migration utility and MySQL/Postgres migration runners for the owner-aggregation retarget. - Deletes the associated unit test for the migration behavior.
- Drops the no-op
1.13.2native migration SQL files that existed only for version discovery.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v1132/OwnerDisplayNameAggregationScrubTest.java | Removes the unit test validating the owner aggregation field retarget in stored searchSettings. |
| openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v1132/MigrationUtil.java | Removes the utility responsible for retargeting stale owner aggregation fields in persisted searchSettings. |
| openmetadata-service/src/main/java/org/openmetadata/service/migration/postgres/v1132/Migration.java | Removes the Postgres migration runner that invoked the v1132 data migration. |
| openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v1132/Migration.java | Removes the MySQL migration runner that invoked the v1132 data migration. |
| bootstrap/sql/migrations/native/1.13.2/postgres/schemaChanges.sql | Removes the no-op SQL marker file for the 1.13.2 Postgres migration. |
| bootstrap/sql/migrations/native/1.13.2/mysql/schemaChanges.sql | Removes the no-op SQL marker file for the 1.13.2 MySQL migration. |
|
Contributor
🔴 Playwright Results — 1 failure(s), 24 flaky✅ 4478 passed · ❌ 1 failed · 🟡 24 flaky · ⏭️ 38 skipped
Genuine Failures (failed on all attempts)❌
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #29691.
That migration diagnosed the wrong root cause. The real failure on the affected clusters (1.12.13) is in the reindex alias swap, not in stored searchSettings:
The
remove_indexaction was built withmust_exist, which OpenSearch's_aliasesparser rejects ([remove_index] unknown field [must_exist]→[aliases] failed to parse field [actions], HTTP 400). Every rebuilt index therefore fails to promote — so the canonical*_search_indexaliases never attach, which is what produces the downstream aggregation/field errors that #29691 tried to paper over.That parser bug is already fixed on
mainby #28700 (dropmust_existfromremove_index); it just needs backporting to 1.12.13 (separate PR). The owner-aggregation scrub is unnecessary and adds a phantom1.13.2migration folder, so it's reverted.🤖 Generated with Claude Code
Greptile Summary
This PR reverts #29691, which introduced a
1.13.2data migration to retarget a stale owner-aggregation field (owners.displayName.keyword→ownerDisplayName) in storedsearchSettings. The PR description argues that #29691 treated a symptom rather than the root cause: the real failure on affected 1.12.13 clusters is thatremove_indexalias actions were built withmust_exist, a field that OpenSearch's_aliasesparser rejects, causing every rebuilt index to fail alias promotion and producing the downstream aggregation errors that #29691 tried to fix. That parser bug is already corrected onmainby #28700 and needs backporting to 1.12.x.Migration.javaclasses, theMigrationUtil.javautility, and the accompanying unit test.v1132remain elsewhere in the codebase after the revert, so the framework will not encounter orphaned entries.Confidence Score: 5/5
Safe to merge. This is a clean revert of a migration that was not yet part of any released version, with no orphaned registry entries remaining.
All six files added by #29691 are deleted consistently. A codebase-wide search confirms no
v1132or1.13.2references survive elsewhere, so the migration framework will not encounter orphaned entries or broken class lookups. The removed migration was data-only (no DDL), idempotent, and targeted a symptom rather than the root cause, making its removal low-risk. The actual root-cause fix (#28700, dropmust_existfromremove_index) is already present on main.No files require special attention — all deletions are consistent and complete.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Cluster upgrading to 1.12.13] --> B[Reindex triggered] B --> C["OpenSearchIndexManager.swapAliases()"] C --> D{"remove_index action\ncontains must_exist?"} D -->|Yes - pre #28700| E["OpenSearch _aliases parser\nrejects must_exist field\nHTTP 400"] E --> F[alias swap fails] F --> G["canonical *_search_index aliases\nnever attach to rebuilt index"] G --> H["Aggregation queries hit\nstale index/mappings"] H --> I["#29691 attempted fix:\nmigrate stale owner agg field\nin stored searchSettings"] I --> J["Root cause still present\n(wrong fix)"] D -->|No - post #28700 / this PR| K["Alias swap succeeds"] K --> L["Correct index promoted\nwith updated mappings"] L --> M["ownerDisplayName agg\nworks correctly"]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[Cluster upgrading to 1.12.13] --> B[Reindex triggered] B --> C["OpenSearchIndexManager.swapAliases()"] C --> D{"remove_index action\ncontains must_exist?"} D -->|Yes - pre #28700| E["OpenSearch _aliases parser\nrejects must_exist field\nHTTP 400"] E --> F[alias swap fails] F --> G["canonical *_search_index aliases\nnever attach to rebuilt index"] G --> H["Aggregation queries hit\nstale index/mappings"] H --> I["#29691 attempted fix:\nmigrate stale owner agg field\nin stored searchSettings"] I --> J["Root cause still present\n(wrong fix)"] D -->|No - post #28700 / this PR| K["Alias swap succeeds"] K --> L["Correct index promoted\nwith updated mappings"] L --> M["ownerDisplayName agg\nworks correctly"]Reviews (1): Last reviewed commit: "Revert "fix(search): migrate stale owner..." | Re-trigger Greptile