LiJie20190102 opened a new pull request, #13094:
URL: https://github.com/apache/gravitino/pull/13094
### What changes were proposed in this pull request?
Schema cascade delete can race with concurrent child metadata writes and
leave active orphan rows in version tables. This change ensures every child
update transaction acquires the parent schema shared lock
(`lockSchemaForEntityWrite`) as its first operation, so a cascade delete cannot
interleave with an in-flight child write.
Changes:
- **TableMetaService.updateTable**: Always lock the parent schema row
unconditionally (previously only locked when `isSchemaChanged=true`). For
cross-schema moves, lock both source and destination schemas in a consistent
order (by schemaId) to prevent deadlocks.
- **FunctionMetaService.updateFunction**: Same pattern — unconditional
schema lock for same-schema updates, dual-lock for cross-schema moves.
- **ViewMetaService.updateView**: Same pattern.
- **FilesetMetaService.updateFileset**: Add schema lock (previously had
none). Inline the old `tryUpdateFileset` helper into `doMultipleWithCommit`
with lock as the first step, and deduplicate `retryFilesetPO` construction (was
built twice; now built once and shared by both lambdas).
- **TopicMetaService.updateTopic**: Add schema lock (previously had none).
- **ModelMetaService.updateModel**: Add schema lock (previously had none;
only `insertModel` had it).
New tests in `TestSchemaMetaService`:
1. `testSchemaChildUpdateServicesWaitForConcurrentSchemaDelete` — runs
cascade delete concurrently with each child type's update (table, view,
fileset, function, model, topic) and asserts the schema and child entity are
gone afterward.
2. `testCascadeDeleteLeavesNoOrphanVersionRows` — runs cascade delete
concurrently with table update, asserts no active `table_version_info` rows
whose parent `table_meta` is deleted (skipped on H2 which lacks `FOR SHARE`;
enforced on MySQL/PostgreSQL).
### Why are the changes needed?
The insert paths (`insertTable`, `insertFunction`, `insertView`, etc.)
already acquire a shared lock on the parent schema row before writing. However,
the corresponding update paths did not consistently do so:
- `updateTable`/`updateFunction`/`updateView`: only locked the schema when
the entity moved to a different schema (`isSchemaChanged=true`); same-schema
updates skipped the lock entirely.
- `updateFileset`/`updateTopic`/`updateModel`: had no schema lock at all.
As a result, a schema cascade delete could commit while an overlapping
writer left an active row in a child or version table whose schema or parent
entity had already been deleted (orphan row).
Fix: #12406
### Does this PR introduce _any_ user-facing change?
No. The fix is an internal concurrency-safety improvement to the relational
storage layer. No public API, REST endpoint, or behavior contract changes.
### How was this patch tested?
- `./gradlew :core:spotlessApply :core:compileJava :core:compileTestJava
-PskipWeb=true`
- `./gradlew :core:test --tests
"org.apache.gravitino.storage.relational.service.TestSchemaMetaService" --tests
"org.apache.gravitino.storage.relational.service.TestModelMetaService" --tests
"org.apache.gravitino.storage.relational.service.TestFilesetMetaService"
-PskipWeb=true -PskipDockerTests=true`
All existing and new tests pass on H2. The no-orphan-row assertion in
`testCascadeDeleteLeavesNoOrphanVersionRows` is gated to non-H2 backends
because H2 lacks `FOR SHARE` (falls back to `FOR UPDATE`); MySQL and PostgreSQL
coverage depends on CI with `dockerTest=true`.
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): Support xxx"
- "[#233] fix: Check null before access result in xxx"
- "[MINOR] refactor: Fix typo in variable name"
- "[MINOR] docs: Fix typo in README"
- "[#255] test: Fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
