Skip to content

Commit 3d40252

Browse files
harshachmohityadav766
harshach
authored and
mohityadav766
committed
fix(search): derive reindex promotion aliases from IndexMapping (#28719) (#28721)
finalizeReindex built its alias-to-attach set from context.getExistingAliases(), which is empty when the ReindexContext is reconstructed on a participant server via fromStagedIndexMapping. That caused promotion to abort without attaching any alias, dropping the parent aliases (all/table/dataAsset) and silently removing the promoted index (most visibly tableColumn/column_search_index) from the dataAsset/global-search alias. Re-derive the alias set from the IndexMapping via getAliasesFromMapping, matching promoteEntityIndex, so promotion is deterministic regardless of how the context was constructed. (cherry picked from commit f623473)
1 parent a346781 commit 3d40252

2 files changed

Lines changed: 127 additions & 33 deletions

File tree

‎openmetadata-service/src/main/java/org/openmetadata/service/search/DefaultRecreateHandler.java‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,10 @@ public void finalizeReindex(EntityReindexContext context, boolean reindexSuccess
8787
String entityType = context.getEntityType();
8888
String canonicalIndex = context.getCanonicalIndex();
8989
String stagedIndex = context.getStagedIndex();
90-
Set<String> aliasesFromMapping = context.getExistingAliases();
9190

9291
SearchRepository searchRepository = Entity.getSearchRepository();
9392
SearchClient searchClient = searchRepository.getSearchClient();
93+
IndexMapping indexMapping = searchRepository.getIndexMapping(entityType);
9494

9595
if (canonicalIndex == null || stagedIndex == null) {
9696
LOG.error(
@@ -144,15 +144,14 @@ public void finalizeReindex(EntityReindexContext context, boolean reindexSuccess
144144
// is strictly worse than the writes going back to the canonical
145145
// alias target. Operators need to retry the reindex either way.
146146
try {
147-
// The alias set was derived from indexMapping.json at recreate time via
148-
// getAliasesFromMapping; finalize just attaches that captured set. Nothing is read from the
149-
// live cluster, so the set is deterministic and matches the distributed promotion path.
150-
Set<String> aliasesToAttach = new HashSet<>();
151-
if (aliasesFromMapping != null) {
152-
aliasesFromMapping.stream()
153-
.filter(alias -> alias != null && !alias.isBlank())
154-
.forEach(aliasesToAttach::add);
155-
}
147+
// Re-derive aliases from indexMapping.json rather than trusting
148+
// context.getExistingAliases(). A participant server rebuilds the context
149+
// from only the staged-index mapping (ReindexContext.fromStagedIndexMapping),
150+
// leaving its alias set empty; trusting it would abort promotion and drop the
151+
// parent aliases (all/table/dataAsset). Deriving from the mapping matches
152+
// promoteEntityIndex.
153+
Set<String> aliasesToAttach =
154+
getAliasesFromMapping(indexMapping, searchRepository.getClusterAlias());
156155

157156
if (aliasesToAttach.isEmpty()) {
158157
abortPromotionWithoutAliases(entityType, stagedIndex);

‎openmetadata-service/src/test/java/org/openmetadata/service/search/DefaultRecreateHandlerTest.java‎

Lines changed: 118 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,15 @@ void testFinalizeReindexPromotesPartialData() {
587587

588588
SearchRepository repo = mock(SearchRepository.class);
589589
when(repo.getSearchClient()).thenReturn(client);
590+
when(repo.getClusterAlias()).thenReturn("");
591+
when(repo.getIndexMapping("table"))
592+
.thenReturn(
593+
IndexMapping.builder()
594+
.indexName("table_search_index")
595+
.alias("table")
596+
.parentAliases(List.of("all"))
597+
.childAliases(List.of())
598+
.build());
590599

591600
ReindexingMetrics metrics = mock(ReindexingMetrics.class);
592601
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class);
@@ -600,9 +609,6 @@ void testFinalizeReindexPromotesPartialData() {
600609
.canonicalIndex("table_search_index")
601610
.activeIndex("table_search_index")
602611
.stagedIndex("table_search_index_rebuild_new")
603-
.existingAliases(new HashSet<>(List.of("legacyAlias", "table", "all", "", " ")))
604-
.canonicalAliases("table")
605-
.parentAliases(new HashSet<>(List.of("all", "", " ")))
606612
.build();
607613

608614
new DefaultRecreateHandler().finalizeReindex(context, false);
@@ -611,8 +617,8 @@ void testFinalizeReindexPromotesPartialData() {
611617
assertTrue(aliasState.deletedIndices.contains("table_search_index"));
612618
assertTrue(aliasState.deletedIndices.contains("table_search_index_rebuild_old"));
613619
Set<String> stagedAliases = aliasState.indexAliases.get("table_search_index_rebuild_new");
614-
assertTrue(stagedAliases.contains("legacyAlias"));
615620
assertTrue(stagedAliases.contains("table"));
621+
assertTrue(stagedAliases.contains("table_search_index"));
616622
assertTrue(stagedAliases.contains("all"));
617623
verify(metrics).recordPromotionSuccess("table");
618624
}
@@ -637,8 +643,6 @@ void testFinalizeReindexDeletesEmptyFailedStage() {
637643
.entityType("table")
638644
.canonicalIndex("table_search_index")
639645
.stagedIndex("table_search_index_rebuild_new")
640-
.existingAliases(new HashSet<>())
641-
.parentAliases(new HashSet<>())
642646
.build();
643647

644648
new DefaultRecreateHandler().finalizeReindex(context, false);
@@ -659,6 +663,15 @@ void testFinalizeReindexStopsWhenAliasSwapFails() {
659663

660664
SearchRepository repo = mock(SearchRepository.class);
661665
when(repo.getSearchClient()).thenReturn(client);
666+
when(repo.getClusterAlias()).thenReturn("");
667+
when(repo.getIndexMapping("table"))
668+
.thenReturn(
669+
IndexMapping.builder()
670+
.indexName("table_search_index")
671+
.alias("table")
672+
.parentAliases(List.of("all"))
673+
.childAliases(List.of())
674+
.build());
662675

663676
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class)) {
664677
entityMock.when(Entity::getSearchRepository).thenReturn(repo);
@@ -668,9 +681,6 @@ void testFinalizeReindexStopsWhenAliasSwapFails() {
668681
.entityType("table")
669682
.canonicalIndex("table_search_index")
670683
.stagedIndex("table_search_index_rebuild_new")
671-
.existingAliases(new HashSet<>(Set.of("table")))
672-
.canonicalAliases("table")
673-
.parentAliases(new HashSet<>(Set.of("all")))
674684
.build();
675685

676686
new DefaultRecreateHandler().finalizeReindex(context, true);
@@ -692,6 +702,8 @@ void testFinalizeReindexDoesNotOrphanAliasWhenNoAliasesResolved() {
692702
SearchClient client = aliasState.toMock();
693703
SearchRepository repo = mock(SearchRepository.class);
694704
when(repo.getSearchClient()).thenReturn(client);
705+
when(repo.getClusterAlias()).thenReturn("");
706+
when(repo.getIndexMapping("table")).thenReturn(null);
695707

696708
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class)) {
697709
entityMock.when(Entity::getSearchRepository).thenReturn(repo);
@@ -701,9 +713,6 @@ void testFinalizeReindexDoesNotOrphanAliasWhenNoAliasesResolved() {
701713
.entityType("table")
702714
.canonicalIndex("table_search_index")
703715
.stagedIndex("table_search_index_rebuild_new")
704-
.existingAliases(new HashSet<>(List.of("", " ")))
705-
.canonicalAliases("")
706-
.parentAliases(new HashSet<>(List.of(" ")))
707716
.build();
708717

709718
new DefaultRecreateHandler().finalizeReindex(context, true);
@@ -730,6 +739,15 @@ void testFinalizeReindexRemovesConcreteCanonicalAtomically() {
730739
SearchClient client = aliasState.toMock();
731740
SearchRepository repo = mock(SearchRepository.class);
732741
when(repo.getSearchClient()).thenReturn(client);
742+
when(repo.getClusterAlias()).thenReturn("");
743+
when(repo.getIndexMapping("table"))
744+
.thenReturn(
745+
IndexMapping.builder()
746+
.indexName("table_search_index")
747+
.alias("table")
748+
.parentAliases(List.of("all"))
749+
.childAliases(List.of())
750+
.build());
733751

734752
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class)) {
735753
entityMock.when(Entity::getSearchRepository).thenReturn(repo);
@@ -740,9 +758,6 @@ void testFinalizeReindexRemovesConcreteCanonicalAtomically() {
740758
.canonicalIndex("table_search_index")
741759
.activeIndex("table_search_index")
742760
.stagedIndex("table_search_index_rebuild_new")
743-
.existingAliases(new HashSet<>(Set.of("table", "table_search_index", "all")))
744-
.canonicalAliases("table")
745-
.parentAliases(new HashSet<>(Set.of("all")))
746761
.build();
747762

748763
new DefaultRecreateHandler().finalizeReindex(context, true);
@@ -768,6 +783,15 @@ void testFinalizeReindexFailedSwapDoesNotOrphanConcreteCanonical() {
768783

769784
SearchRepository repo = mock(SearchRepository.class);
770785
when(repo.getSearchClient()).thenReturn(client);
786+
when(repo.getClusterAlias()).thenReturn("");
787+
when(repo.getIndexMapping("table"))
788+
.thenReturn(
789+
IndexMapping.builder()
790+
.indexName("table_search_index")
791+
.alias("table")
792+
.parentAliases(List.of("all"))
793+
.childAliases(List.of())
794+
.build());
771795

772796
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class)) {
773797
entityMock.when(Entity::getSearchRepository).thenReturn(repo);
@@ -778,9 +802,6 @@ void testFinalizeReindexFailedSwapDoesNotOrphanConcreteCanonical() {
778802
.canonicalIndex("table_search_index")
779803
.activeIndex("table_search_index")
780804
.stagedIndex("table_search_index_rebuild_new")
781-
.existingAliases(new HashSet<>(Set.of("table", "table_search_index", "all")))
782-
.canonicalAliases("table")
783-
.parentAliases(new HashSet<>(Set.of("all")))
784805
.build();
785806

786807
new DefaultRecreateHandler().finalizeReindex(context, true);
@@ -801,6 +822,10 @@ void testFinalizeReindexRecordsPromotionFailureOnException() {
801822

802823
SearchRepository repo = mock(SearchRepository.class);
803824
when(repo.getSearchClient()).thenReturn(client);
825+
when(repo.getClusterAlias()).thenReturn("");
826+
when(repo.getIndexMapping("table"))
827+
.thenReturn(
828+
IndexMapping.builder().indexName("table_search_index").alias("table").build());
804829

805830
ReindexingMetrics metrics = mock(ReindexingMetrics.class);
806831
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class);
@@ -813,8 +838,6 @@ void testFinalizeReindexRecordsPromotionFailureOnException() {
813838
.entityType("table")
814839
.canonicalIndex("table_search_index")
815840
.stagedIndex("table_search_index_rebuild_new")
816-
.existingAliases(new HashSet<>(Set.of("table")))
817-
.parentAliases(new HashSet<>())
818841
.build();
819842

820843
new DefaultRecreateHandler().finalizeReindex(context, true);
@@ -845,6 +868,15 @@ void testFinalizeReindexSkipsDeleteWhenCanonicalIsAlias() {
845868
SearchClient client = aliasState.toMock();
846869
SearchRepository repo = mock(SearchRepository.class);
847870
when(repo.getSearchClient()).thenReturn(client);
871+
when(repo.getClusterAlias()).thenReturn("");
872+
when(repo.getIndexMapping("table"))
873+
.thenReturn(
874+
IndexMapping.builder()
875+
.indexName("table_search_index")
876+
.alias("table")
877+
.parentAliases(List.of("all"))
878+
.childAliases(List.of())
879+
.build());
848880

849881
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class)) {
850882
entityMock.when(Entity::getSearchRepository).thenReturn(repo);
@@ -855,9 +887,6 @@ void testFinalizeReindexSkipsDeleteWhenCanonicalIsAlias() {
855887
.canonicalIndex("table_search_index")
856888
.activeIndex("table_search_index_rebuild_old")
857889
.stagedIndex("table_search_index_rebuild_new")
858-
.existingAliases(new HashSet<>(Set.of("table_search_index", "table", "all")))
859-
.canonicalAliases("table")
860-
.parentAliases(new HashSet<>(Set.of("all")))
861890
.build();
862891

863892
new DefaultRecreateHandler().finalizeReindex(context, true);
@@ -878,6 +907,72 @@ void testFinalizeReindexSkipsDeleteWhenCanonicalIsAlias() {
878907
stagedAliases.contains("all"),
879908
() -> "Parent alias must end up on staged after promotion; got " + stagedAliases);
880909
}
910+
911+
@Test
912+
@DisplayName(
913+
"Should attach parent aliases from the mapping when the context carries none "
914+
+ "(participant-reconstructed context)")
915+
void testFinalizeReindexDerivesAliasesFromMappingWhenContextHasNoAliases() {
916+
// Regression for the distributed-reindex alias-loss bug: a participant server reconstructs
917+
// the
918+
// ReindexContext from only the job's staged-index mapping (ReindexContext
919+
// .fromStagedIndexMapping), so existingAliases/parentAliases are empty. finalizeReindex must
920+
// still attach the column index's parent aliases (all/table/dataAsset) by re-deriving them
921+
// from indexMapping.json — otherwise the promoted column index drops out of the
922+
// dataAsset/global-search alias. Before the fix, finalize trusted
923+
// context.getExistingAliases()
924+
// (empty here) and aborted promotion without attaching any alias.
925+
AliasState aliasState = new AliasState();
926+
aliasState.put(
927+
"column_search_index_rebuild_old",
928+
new HashSet<>(Set.of("tableColumn", "column_search_index", "all", "table", "dataAsset")));
929+
aliasState.put("column_search_index_rebuild_new", new HashSet<>());
930+
931+
SearchClient client = aliasState.toMock();
932+
SearchRepository repo = mock(SearchRepository.class);
933+
when(repo.getSearchClient()).thenReturn(client);
934+
when(repo.getClusterAlias()).thenReturn("");
935+
when(repo.getIndexMapping("tableColumn"))
936+
.thenReturn(
937+
IndexMapping.builder()
938+
.indexName("column_search_index")
939+
.alias("tableColumn")
940+
.parentAliases(List.of("all", "table", "dataAsset"))
941+
.childAliases(List.of())
942+
.build());
943+
944+
try (MockedStatic<Entity> entityMock = mockStatic(Entity.class)) {
945+
entityMock.when(Entity::getSearchRepository).thenReturn(repo);
946+
947+
EntityReindexContext context =
948+
EntityReindexContext.builder()
949+
.entityType("tableColumn")
950+
.canonicalIndex("column_search_index")
951+
.stagedIndex("column_search_index_rebuild_new")
952+
.existingAliases(new HashSet<>())
953+
.parentAliases(new HashSet<>())
954+
.build();
955+
956+
new DefaultRecreateHandler().finalizeReindex(context, true);
957+
}
958+
959+
Set<String> stagedAliases = aliasState.indexAliases.get("column_search_index_rebuild_new");
960+
assertTrue(stagedAliases.contains("tableColumn"));
961+
assertTrue(stagedAliases.contains("column_search_index"));
962+
assertTrue(
963+
stagedAliases.contains("all"),
964+
() -> "parent alias 'all' must be derived from the mapping; got " + stagedAliases);
965+
assertTrue(
966+
stagedAliases.contains("table"),
967+
() -> "parent alias 'table' must be derived from the mapping; got " + stagedAliases);
968+
assertTrue(
969+
stagedAliases.contains("dataAsset"),
970+
() ->
971+
"parent alias 'dataAsset' must be derived from the mapping — this is the regression; "
972+
+ "got "
973+
+ stagedAliases);
974+
assertTrue(aliasState.deletedIndices.contains("column_search_index_rebuild_old"));
975+
}
881976
}
882977

883978
@Nested

0 commit comments

Comments
 (0)