fix(migration): v200 thread_entity guard, PostgreSQL jsonb cast, self-loop cycle detection#27756
fix(migration): v200 thread_entity guard, PostgreSQL jsonb cast, self-loop cycle detection#27756
Conversation
…-loop cycle detection - Add tableExists guard to migrateThreadTasksToTaskEntity so it skips safely when thread_entity has been renamed to thread_entity_legacy by postDataMigration SQL, preventing crashes on fresh installs and migration re-runs - Fix PostgreSQL jsonb type mismatch in migrateSuggestionsToTaskEntity and migrateLegacyActivityThreadsToActivityStream: JDBI binds String as varchar which PostgreSQL rejects for jsonb columns; use ::jsonb cast when ConnectionType is POSTGRES - Re-enable migrateThreadTasksToTaskEntity for MySQL (was commented out as workaround) now that the tableExists guard makes it safe - Fix hasCycleDFS in WorkflowDefinitionRepository to skip self-loop edges so BPMN reassignment cycles (AssignedStage -> AssignedStage) are not flagged as cycles, allowing IncidentLifecycleWorkflow to seed without error - Fix initSeedDataFromResources in EntityRepository to continue seeding remaining entities when one entity fails, instead of aborting the entire seed operation - Add unit tests covering MySQL/PostgreSQL variants for all three migration methods, tableExists guard, and jsonb cast behavior (13 tests total, all passing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
yan-3005
added
backend
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR hardens v200 migrations and seed/workflow initialization by making migrations resilient to schema drift (renamed tables), fixing PostgreSQL JSONB inserts, and relaxing workflow validation to allow intentional self-loop transitions.
Changes:
- Add v200 migration guards and PostgreSQL
::jsonbcasts for task/activity inserts viaConnectionType. - Re-enable MySQL thread-task migration with a
thread_entityexistence guard. - Improve workflow seeding robustness by skipping self-loop edges in cycle detection and continuing seed initialization after per-entity failures.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v200/MigrationUtil.java | Adds ConnectionType-aware ::jsonb binding and a thread_entity existence guard; threads/suggestions/activity migrations now pass connection type into inserts. |
| openmetadata-service/src/main/java/org/openmetadata/service/migration/postgres/v200/Migration.java | Updates v200 Postgres migration to pass POSTGRES into updated MigrationUtil APIs. |
| openmetadata-service/src/main/java/org/openmetadata/service/migration/mysql/v200/Migration.java | Updates v200 MySQL migration to pass MYSQL and re-enables thread-task migration call. |
| openmetadata-service/src/test/java/org/openmetadata/service/migration/utils/v200/MigrationUtilTest.java | Adds tests for table-missing guards and attempts to cover Postgres/MySQL jsonb casting behavior. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowDefinitionRepository.java | Skips self-loop edges during DFS cycle detection to allow valid reassignment transitions. |
| openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java | Makes seed initialization resilient by catching/logging per-entity failures instead of aborting all seeding. |
| void migrateThreadTasksUsesJsonbCastForPostgres() { | ||
| when(handle.createQuery("SELECT 1 FROM thread_entity LIMIT 1").mapTo(Integer.class).one()) | ||
| .thenReturn(1); | ||
| when(handle | ||
| .createQuery( | ||
| "SELECT json FROM thread_entity WHERE type = 'Task' ORDER BY createdAt ASC") | ||
| .mapToMap() | ||
| .list()) | ||
| .thenReturn(java.util.List.of()); | ||
|
|
||
| assertDoesNotThrow(() -> MigrationUtil.migrateThreadTasksToTaskEntity(handle, POSTGRES)); | ||
|
|
||
| verify(handle, never()).createUpdate(contains(":json,")); | ||
| } |
There was a problem hiding this comment.
migrateThreadTasksUsesJsonbCastForPostgres doesn’t currently validate the ::jsonb insert behavior because the mocked thread list is empty, so insertTask(...) is never reached and Handle.createUpdate(...) is never called. Consider returning a non-empty threads result (one row with a minimal json payload) and then asserting that the SQL passed to createUpdate contains :json::jsonb for POSTGRES.
… tests - Fix tableExists to use findFirst() instead of one(): one() throws on an empty result set (0 rows from an existing but empty table), causing the guard to incorrectly return false and log "table does not exist" - Replace vacuous jsonb cast tests with direct insertTask reflection tests: the previous tests mocked an empty thread list so insertTask was never reached, making assertions trivially true regardless of cast logic; now call insertTask directly via reflection so the ::jsonb cast path is actually exercised for POSTGRES and absent for MYSQL - Update tableExists mock stubs in tests from .one() to .findFirst() to match the corrected implementation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@gitar-bot Intentional design choice — each failure is already individually logged with full context (entity type, FQN, message, and full stack trace) via |
That's a fair point — individual WARN logs with full context per entity do provide better diagnostic coverage than surfacing a single arbitrary |
Code Review 👍 Approved with suggestions 1 resolved / 2 findingsImplements v200 thread entity guards and robust PostgreSQL jsonb casting, resolving previous issues with vacuous test results. Consider refining the seed data initialization to avoid swallowing errors that could mask underlying system failures. 💡 Edge Case: Seed data failures silently swallowed may hide systemic issues📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/EntityRepository.java:1231-1240 The new try/catch in Suggested fix✅ 1 resolved✅ Quality: Jsonb cast tests pass vacuously with empty input data
🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
🔴 Playwright Results — 2 failure(s), 17 flaky✅ 3957 passed · ❌ 2 failed · 🟡 17 flaky · ⏭️ 86 skipped
Genuine Failures (failed on all attempts)❌
|
Context
This PR fixes five bugs introduced or exposed during the v2.0.0 migration, all of which caused CI failures on both MySQL and PostgreSQL. The bugs compound each other: the workflow seeding failures (#4 and #5) were only visible because the migration bugs (#1–3) caused the server to restart mid-migration and hit edge cases on re-run.
Bug 1:
thread_entitycrash on migration re-run (MySQL + PostgreSQL)Root cause: OpenMetadata uses two independent tracking systems for migrations:
flyway_schema_history— each script is committed atomically and cannot be rolled back once applied.server_change_loginside a transaction — if a later migration step fails, the transaction rolls back and the Java state reverts to "v200 not yet run."The v2.0.0
postDataMigrationSQLScript.sqlrenamesthread_entity → thread_entity_legacyafter the Java data migration runs. If any subsequent migration throws and rolls back theserver_change_logtransaction, Flyway has already committed the rename permanently. On the next server startup, Java re-executesmigrateThreadTasksToTaskEntity, which tries toSELECT FROM thread_entity— a table that no longer exists — and crashes.This also affects upgrades from certain versions where
thread_entitywas never present.Fix: Guard
migrateThreadTasksToTaskEntitywith atableExists(handle, "thread_entity")check at the top of the method and return early if the table is absent.Bug 2: PostgreSQL
jsonbtype mismatch — all 1032 suggestion inserts failRoot cause: The v2.0.0 PostgreSQL schema declares the following columns as
jsonb NOT NULL:task_entity.jsonactivity_stream.jsonactivity_stream.domainsJDBI's
.bind("fieldName", someString)always binds a JavaStringas SQL typevarchar. PostgreSQL enforces strict type checking and does not implicitly castvarchartojsonb, throwing:MySQL's
jsontype is lenient and accepts string input without an explicit cast, so MySQL never surfaced this bug. PostgreSQL's strictness caused every single suggestion insert and every activity event insert to fail.Fix: Pass
ConnectionTypethrough the call chain intoinsertTaskandinsertActivityEvent. WhenConnectionType.POSTGRES, use an explicit::jsonbcast in the SQL string (e.g.,:json::jsonb). MySQL continues to use the plain:jsonbinding.Bug 3: MySQL
migrateThreadTasksToTaskEntitysilently disabledRoot cause: The MySQL migration had
migrateThreadTasksToTaskEntitycommented out as a temporary workaround after it caused CI failures:The actual root cause was the
thread_entitycrash described in Bug 1 — not the method itself. This meant MySQL deployments silently skipped migrating thread-based tasks totask_entity, resulting in missing task data for all MySQL installations.Fix: Re-enable the method now that Bug 1's
tableExistsguard makes it safe to call. The commented-out code is removed.Bug 4: BPMN self-loop edges falsely detected as cycles
Root cause:
hasCycleDFSinWorkflowDefinitionRepositoryiterates over all outgoing edges of a node and recurses into each neighbor. It does not skip self-loop edges — edges where a node transitions to itself (e.g.,AssignedStage → AssignedStage).BPMN self-loops are a valid and intentional pattern for reassignment transitions: an incident can be reassigned to a different team while remaining in the
AssignedStagestate. The DFS treated this as a cycle and threw a validation exception whenIncidentLifecycleWorkflowwas seeded.Fix: Skip the recursion when
neighbor.equals(node)— a self-loop cannot form a multi-node cycle.Bug 5: One failing seed entity aborts all subsequent seeds
Root cause:
initSeedDataFromResourcesinEntityRepositoryiterates over workflow definitions alphabetically. Seed files are processed in alphabetical order, soIncidentLifecycleWorkflowis initialized beforeTableCertificationWorkflow. When Bug 4 causedIncidentLifecycleWorkflowinitialization to throw an exception, the exception propagated out of the loop uncaught, aborting the entire seed operation.TableCertificationWorkflowand every other definition that came after were never seeded.Fix: Wrap each entity initialization in a try-catch. Log the failure with context (entity type, FQN, error message and stack trace) and continue to the next entity. One bad workflow definition should not prevent all others from loading.
Tests
Added 7 new unit tests in
MigrationUtilTestcovering:tableExistsguard skips gracefully formigrateThreadTasksToTaskEntity(MySQL + PostgreSQL)tableExistsguard skips gracefully formigrateSuggestionsToTaskEntity(MySQL + PostgreSQL)tableExistsguard skips gracefully formigrateLegacyActivityThreadsToActivityStream(MySQL + PostgreSQL)migrateThreadTasksToTaskEntityuses::jsonbcast for PostgreSQLmigrateThreadTasksToTaskEntitydoes not use::jsonbcast for MySQLAll 13 tests in the file pass.
Test plan
thread_entity does not existcrashcolumn "json" is of type jsonb but expression is of type character varyingMigrationUtilTestpass (13 total)IncidentLifecycleWorkflowseeds successfully on fresh installTableCertificationWorkflowseeds successfully on fresh installmvn spotless:apply -pl openmetadata-servicereports no formatting issues🤖 Generated with Claude Code