Skip to content

Commit 26d5ae1

Browse files
manerowpmbrull
authored andcommitted
fix(migration): heal stuck PG certification stranded by v1125 (v11210) (#28635)
* fix(migration): heal stuck PG certification stranded by v1125 in v11210 * test(migration): assert v11210 heal is no-op on 1.13 reprocess (idempotent insert)
1 parent cc70a45 commit 26d5ae1

4 files changed

Lines changed: 327 additions & 0 deletions

File tree

openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v11210/Migration.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import lombok.SneakyThrows;
1616
import lombok.extern.slf4j.Slf4j;
17+
import org.openmetadata.service.jdbi3.locator.ConnectionType;
1718
import org.openmetadata.service.migration.api.MigrationProcessImpl;
1819
import org.openmetadata.service.migration.utils.MigrationFile;
1920
import org.openmetadata.service.migration.utils.v11210.MigrationUtil;
@@ -30,5 +31,10 @@ public Migration(MigrationFile migrationFile) {
3031
public void runDataMigration() {
3132
MigrationUtil.removeFlattenedChildrenSearchSettings();
3233
MigrationUtil.removeStaleFileExtensionAggregation();
34+
try {
35+
MigrationUtil.healStuckCertificationOnEntityJson(handle, ConnectionType.MYSQL);
36+
} catch (Exception e) {
37+
LOG.error("v11210 heal of stuck certification on entity json failed", e);
38+
}
3339
}
3440
}

openmetadata-service/src/main/java/org/openmetadata/service/migration/postgres/v11210/Migration.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import lombok.SneakyThrows;
1616
import lombok.extern.slf4j.Slf4j;
17+
import org.openmetadata.service.jdbi3.locator.ConnectionType;
1718
import org.openmetadata.service.migration.api.MigrationProcessImpl;
1819
import org.openmetadata.service.migration.utils.MigrationFile;
1920
import org.openmetadata.service.migration.utils.v11210.MigrationUtil;
@@ -30,5 +31,10 @@ public Migration(MigrationFile migrationFile) {
3031
public void runDataMigration() {
3132
MigrationUtil.removeFlattenedChildrenSearchSettings();
3233
MigrationUtil.removeStaleFileExtensionAggregation();
34+
try {
35+
MigrationUtil.healStuckCertificationOnEntityJson(handle, ConnectionType.POSTGRES);
36+
} catch (Exception e) {
37+
LOG.error("v11210 heal of stuck certification on entity json failed", e);
38+
}
3339
}
3440
}

openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v11210/MigrationUtil.java

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,26 @@
1515
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
1616
import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
1717

18+
import com.fasterxml.jackson.databind.node.ObjectNode;
19+
import java.sql.Timestamp;
20+
import java.util.ArrayList;
1821
import java.util.List;
22+
import java.util.Map;
1923
import java.util.Set;
2024
import lombok.extern.slf4j.Slf4j;
25+
import org.jdbi.v3.core.Handle;
26+
import org.jdbi.v3.core.statement.PreparedBatch;
2127
import org.openmetadata.schema.api.search.Aggregation;
2228
import org.openmetadata.schema.api.search.AllowedSearchFields;
2329
import org.openmetadata.schema.api.search.AssetTypeConfiguration;
2430
import org.openmetadata.schema.api.search.FieldBoost;
2531
import org.openmetadata.schema.api.search.SearchSettings;
2632
import org.openmetadata.schema.settings.Settings;
33+
import org.openmetadata.schema.type.TagLabel;
34+
import org.openmetadata.schema.utils.JsonUtils;
35+
import org.openmetadata.service.jdbi3.locator.ConnectionType;
2736
import org.openmetadata.service.migration.utils.SearchSettingsMergeUtil;
37+
import org.openmetadata.service.util.FullyQualifiedName;
2838

2939
@Slf4j
3040
public class MigrationUtil {
@@ -164,4 +174,209 @@ private static boolean removeStaleSearchField(List<FieldBoost> searchFields) {
164174
}
165175
return removed;
166176
}
177+
178+
// Entity tables that v1125 attempted to drain of inline certification into tag_usage.
179+
private static final String[] CERTIFIED_ENTITY_TABLES = {
180+
"table_entity",
181+
"dashboard_entity",
182+
"topic_entity",
183+
"pipeline_entity",
184+
"storage_container_entity",
185+
"search_index_entity",
186+
"ml_model_entity",
187+
"stored_procedure_entity",
188+
"dashboard_data_model_entity",
189+
"api_endpoint_entity",
190+
"api_collection_entity",
191+
"database_entity",
192+
"database_schema_entity",
193+
"data_product_entity",
194+
"domain_entity",
195+
"chart_entity",
196+
"metric_entity",
197+
"file_entity",
198+
"directory_entity",
199+
"spreadsheet_entity",
200+
"worksheet_entity",
201+
"llm_model_entity",
202+
"ai_application_entity"
203+
};
204+
205+
private static final int HEAL_BATCH_SIZE = 500;
206+
private static final int CERT_SOURCE = TagLabel.TagSource.CLASSIFICATION.ordinal();
207+
private static final int CERT_LABEL_TYPE = TagLabel.LabelType.AUTOMATED.ordinal();
208+
private static final int CERT_STATE = TagLabel.State.CONFIRMED.ordinal();
209+
210+
/**
211+
* Heals installs where certification is still inlined in entity JSON instead of being in
212+
* tag_usage. v1125 on PostgreSQL silently failed on a missing varchar-&gt;json cast for the
213+
* metadata column: the insert threw, the per-table catch swallowed it, and v1125 was still
214+
* recorded as DONE — stranding certification in entity JSON with no tag_usage row. v1125 never
215+
* re-runs, so this heal (self-contained with the correct cast, independent of the v1125 SQL)
216+
* recovers those clusters. Idempotent: the select only matches rows that still carry
217+
* certification in JSON, so fresh installs, healthy MySQL, and already-healed PG databases exit
218+
* at zero rows.
219+
*/
220+
public static void healStuckCertificationOnEntityJson(Handle handle, ConnectionType connType) {
221+
int totalHealed = 0;
222+
for (String table : CERTIFIED_ENTITY_TABLES) {
223+
try {
224+
int healed = healCertificationForTable(handle, connType, table);
225+
totalHealed += healed;
226+
if (healed > 0) {
227+
LOG.info("v11210 heal: moved {} stuck certification rows from {}", healed, table);
228+
}
229+
} catch (Exception e) {
230+
// Per-table failure shouldn't stop the rest; logged at ERROR so it isn't lost.
231+
LOG.error("v11210 heal failed for table '{}': {}", table, e.getMessage(), e);
232+
}
233+
}
234+
LOG.info("v11210 heal: total stuck certification rows healed: {}", totalHealed);
235+
}
236+
237+
private record BatchResult(int rowsFetched, int healed) {}
238+
239+
private static int healCertificationForTable(
240+
Handle handle, ConnectionType connType, String table) {
241+
int totalHealed = 0;
242+
boolean morePages = true;
243+
while (morePages) {
244+
BatchResult batch = healCertificationBatch(handle, connType, table);
245+
totalHealed += batch.healed();
246+
morePages = batch.rowsFetched() == HEAL_BATCH_SIZE;
247+
}
248+
return totalHealed;
249+
}
250+
251+
private static BatchResult healCertificationBatch(
252+
Handle handle, ConnectionType connType, String table) {
253+
boolean isPostgres = connType == ConnectionType.POSTGRES;
254+
int healed = 0;
255+
List<Map<String, Object>> rows =
256+
handle.createQuery(buildSelectStuckCertificationSql(table, isPostgres)).mapToMap().list();
257+
if (!nullOrEmpty(rows)) {
258+
List<String> selectedIds = new ArrayList<>();
259+
PreparedBatch batch = handle.prepareBatch(buildInsertTagUsageSql(isPostgres));
260+
for (Map<String, Object> row : rows) {
261+
String tagFQN = (String) row.get("tagfqn");
262+
// SELECT already filters tagFQN NOT NULL, but stay defensive against concurrent edits.
263+
if (tagFQN != null) {
264+
selectedIds.add(row.get("id").toString());
265+
batch
266+
.bind("source", CERT_SOURCE)
267+
.bind("tagFQN", tagFQN)
268+
.bind("tagFQNHash", FullyQualifiedName.buildHash(tagFQN))
269+
.bind("targetFQNHash", row.get("fqnhash").toString())
270+
.bind("labelType", CERT_LABEL_TYPE)
271+
.bind("state", CERT_STATE)
272+
.bind("appliedAt", parseEpochMillisToTimestamp(row.get("applieddate")))
273+
.bind("metadata", buildCertMetadataJson(row.get("expirydate")))
274+
.add();
275+
}
276+
}
277+
if (!nullOrEmpty(selectedIds)) {
278+
batch.execute();
279+
handle
280+
.createUpdate(buildStripCertificationFromJsonSql(table, isPostgres))
281+
.bindList("ids", selectedIds)
282+
.execute();
283+
healed = selectedIds.size();
284+
}
285+
}
286+
return new BatchResult(rows.size(), healed);
287+
}
288+
289+
private static String buildSelectStuckCertificationSql(String table, boolean isPostgres) {
290+
String sql;
291+
if (isPostgres) {
292+
sql =
293+
String.format(
294+
"SELECT id, fqnHash, "
295+
+ "json::json -> 'certification' -> 'tagLabel' ->> 'tagFQN' AS tagFQN, "
296+
+ "json::json -> 'certification' ->> 'expiryDate' AS expiryDate, "
297+
+ "json::json -> 'certification' ->> 'appliedDate' AS appliedDate "
298+
+ "FROM %s WHERE json::jsonb ?? 'certification' "
299+
+ "AND json::json -> 'certification' -> 'tagLabel' ->> 'tagFQN' IS NOT NULL "
300+
+ "LIMIT %d",
301+
table, HEAL_BATCH_SIZE);
302+
} else {
303+
sql =
304+
String.format(
305+
"SELECT id, fqnHash, "
306+
+ "JSON_UNQUOTE(JSON_EXTRACT(json, '$.certification.tagLabel.tagFQN')) AS tagFQN, "
307+
+ "JSON_UNQUOTE(JSON_EXTRACT(json, '$.certification.expiryDate')) AS expiryDate, "
308+
+ "JSON_UNQUOTE(JSON_EXTRACT(json, '$.certification.appliedDate')) AS appliedDate "
309+
+ "FROM %s WHERE JSON_CONTAINS_PATH(json, 'one', '$.certification') = 1 "
310+
+ "AND JSON_UNQUOTE(JSON_EXTRACT(json, '$.certification.tagLabel.tagFQN')) IS NOT NULL "
311+
+ "LIMIT %d",
312+
table, HEAL_BATCH_SIZE);
313+
}
314+
return sql;
315+
}
316+
317+
private static String buildInsertTagUsageSql(boolean isPostgres) {
318+
// PG won't auto-cast varchar -> json on the metadata column; MySQL JSON accepts a string.
319+
String sql;
320+
if (isPostgres) {
321+
sql =
322+
"INSERT INTO tag_usage "
323+
+ "(source, tagFQN, tagFQNHash, targetFQNHash, labelType, state, appliedBy, appliedAt, metadata) "
324+
+ "VALUES (:source, :tagFQN, :tagFQNHash, :targetFQNHash, :labelType, :state, 'admin', :appliedAt, :metadata::json) "
325+
+ "ON CONFLICT (source, tagFQNHash, targetFQNHash) DO NOTHING";
326+
} else {
327+
sql =
328+
"INSERT IGNORE INTO tag_usage "
329+
+ "(source, tagFQN, tagFQNHash, targetFQNHash, labelType, state, appliedBy, appliedAt, metadata) "
330+
+ "VALUES (:source, :tagFQN, :tagFQNHash, :targetFQNHash, :labelType, :state, 'admin', :appliedAt, :metadata)";
331+
}
332+
return sql;
333+
}
334+
335+
private static String buildStripCertificationFromJsonSql(String table, boolean isPostgres) {
336+
String sql;
337+
if (isPostgres) {
338+
sql =
339+
"UPDATE "
340+
+ table
341+
+ " SET json = (json::jsonb - 'certification')::json WHERE id IN (<ids>)";
342+
} else {
343+
sql =
344+
"UPDATE "
345+
+ table
346+
+ " SET json = JSON_REMOVE(json, '$.certification') WHERE id IN (<ids>)";
347+
}
348+
return sql;
349+
}
350+
351+
private static Timestamp parseEpochMillisToTimestamp(Object val) {
352+
Timestamp result = null;
353+
if (val != null) {
354+
try {
355+
long epochMillis =
356+
val instanceof Number num ? num.longValue() : Long.parseLong(val.toString());
357+
result = new Timestamp(epochMillis);
358+
} catch (NumberFormatException e) {
359+
LOG.warn("Unparseable appliedDate '{}' on heal; binding null", val);
360+
}
361+
}
362+
return result;
363+
}
364+
365+
private static String buildCertMetadataJson(Object expiryDateVal) {
366+
ObjectNode node = JsonUtils.getObjectNode();
367+
if (expiryDateVal != null) {
368+
if (expiryDateVal instanceof Long longVal) {
369+
node.put("expiryDate", longVal);
370+
} else if (expiryDateVal instanceof Number numberVal) {
371+
node.put("expiryDate", numberVal.longValue());
372+
} else {
373+
try {
374+
node.put("expiryDate", Long.parseLong(expiryDateVal.toString()));
375+
} catch (NumberFormatException e) {
376+
LOG.warn("Unparseable expiryDate '{}' on heal; omitting from metadata", expiryDateVal);
377+
}
378+
}
379+
}
380+
return node.toString();
381+
}
167382
}
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*
2+
* Copyright 2024 Collate.
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
package org.openmetadata.service.migration.utils.v11210;
14+
15+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
16+
import static org.junit.jupiter.api.Assertions.assertFalse;
17+
import static org.junit.jupiter.api.Assertions.assertTrue;
18+
import static org.mockito.Answers.RETURNS_DEEP_STUBS;
19+
import static org.mockito.ArgumentMatchers.anyString;
20+
import static org.mockito.Mockito.atLeastOnce;
21+
import static org.mockito.Mockito.mock;
22+
import static org.mockito.Mockito.never;
23+
import static org.mockito.Mockito.verify;
24+
import static org.mockito.Mockito.when;
25+
26+
import java.util.List;
27+
import java.util.Map;
28+
import org.jdbi.v3.core.Handle;
29+
import org.junit.jupiter.api.Test;
30+
import org.mockito.ArgumentCaptor;
31+
import org.openmetadata.service.jdbi3.locator.ConnectionType;
32+
33+
class MigrationUtilTest {
34+
35+
private static Map<String, Object> stuckCertificationRow() {
36+
return Map.of(
37+
"id", "11111111-1111-1111-1111-111111111111",
38+
"fqnhash", "abcdef",
39+
"tagfqn", "Certification.Gold",
40+
"applieddate", 1700000000000L,
41+
"expirydate", 1800000000000L);
42+
}
43+
44+
@Test
45+
void postgresHealInsertCastsMetadataToJson() {
46+
// The #2B regression: on PG the metadata (json) column rejects an uncast varchar bind. The
47+
// heal's insert must cast :metadata to json or it fails the same way v1125 did.
48+
Handle handle = mock(Handle.class, RETURNS_DEEP_STUBS);
49+
when(handle.createQuery(anyString()).mapToMap().list())
50+
.thenReturn(List.of(stuckCertificationRow()));
51+
ArgumentCaptor<String> insertSql = ArgumentCaptor.forClass(String.class);
52+
53+
MigrationUtil.healStuckCertificationOnEntityJson(handle, ConnectionType.POSTGRES);
54+
55+
verify(handle, atLeastOnce()).prepareBatch(insertSql.capture());
56+
assertTrue(
57+
insertSql.getAllValues().stream().allMatch(sql -> sql.contains(":metadata::json")),
58+
"PostgreSQL heal insert must cast :metadata to json");
59+
// Backstop for the 1.13 reprocess: a duplicate insert must be a no-op against the unique key.
60+
assertTrue(
61+
insertSql.getAllValues().stream()
62+
.allMatch(sql -> sql.contains("ON CONFLICT") && sql.contains("DO NOTHING")),
63+
"PostgreSQL heal insert must be idempotent via ON CONFLICT DO NOTHING");
64+
}
65+
66+
@Test
67+
void mysqlHealInsertDoesNotCastMetadata() {
68+
// MySQL JSON columns accept a string literal; the ::json cast is PG-only syntax.
69+
Handle handle = mock(Handle.class, RETURNS_DEEP_STUBS);
70+
when(handle.createQuery(anyString()).mapToMap().list())
71+
.thenReturn(List.of(stuckCertificationRow()));
72+
ArgumentCaptor<String> insertSql = ArgumentCaptor.forClass(String.class);
73+
74+
MigrationUtil.healStuckCertificationOnEntityJson(handle, ConnectionType.MYSQL);
75+
76+
verify(handle, atLeastOnce()).prepareBatch(insertSql.capture());
77+
assertFalse(
78+
insertSql.getAllValues().stream().anyMatch(sql -> sql.contains("::json")),
79+
"MySQL heal insert must not use the PG-only ::json cast");
80+
// Backstop for the 1.13 reprocess: a duplicate insert must be a no-op against the unique key.
81+
assertTrue(
82+
insertSql.getAllValues().stream().allMatch(sql -> sql.contains("INSERT IGNORE")),
83+
"MySQL heal insert must be idempotent via INSERT IGNORE");
84+
}
85+
86+
@Test
87+
void healWithNoStuckRowsRunsNoInsertOrStrip() {
88+
// This is the 1.13-upgrade case: v11210 already healed (and STRIPPED certification from the
89+
// entity JSON) during the 1.12.10 run, so the SELECT now matches zero rows. The framework
90+
// reprocesses v11210 and also runs v1130's heal on that upgrade, but both must issue NO insert
91+
// and NO strip query — the same insert queries from 1.12.10 do not run again.
92+
Handle handle = mock(Handle.class, RETURNS_DEEP_STUBS);
93+
when(handle.createQuery(anyString()).mapToMap().list()).thenReturn(List.of());
94+
95+
assertDoesNotThrow(
96+
() -> MigrationUtil.healStuckCertificationOnEntityJson(handle, ConnectionType.POSTGRES));
97+
verify(handle, never()).prepareBatch(anyString());
98+
verify(handle, never()).createUpdate(anyString());
99+
}
100+
}

0 commit comments

Comments
 (0)