nevzheng opened a new pull request, #14262:
URL: https://github.com/apache/gravitino/pull/14262
### What changes were proposed in this pull request?
- **New `IcebergStaleEntityCleaner`** (package-private). After a successful
backend create,
staged-create commit, register without overwrite, or view create or
register, the hooks delete any
entity left under the name (this cascades its bindings), remove the plugin
privileges for the name,
then import. If the stale entity cannot be deleted, the request fails
instead of importing onto it.
- **`registerTable` with overwrite keeps today's behavior**: an existing
entity is kept with its
bindings.
- **`registerView` no longer keeps an existing entity.** Register view has
no overwrite, so a
successful backend register means no view had the name.
- The drop reconcile stays best-effort, but a failure is now logged at ERROR
instead of WARN.
### Why are the changes needed?
**A new table or view could inherit a dropped object's entity.** Iceberg
REST tables and views carry no
Gravitino id, so the core import adopts any entity left under the name.
After a drop whose entity
cleanup failed, a new object under that name got the old id, and with it the
old role grants, tags,
policies and statistics. Register also kept the old owner.
Fix: #14261
### Does this PR introduce _any_ user-facing change?
Yes. A create or register through Iceberg REST no longer inherits a previous
object's grants, tags or
owner. If the stale entity cannot be removed, the create returns an error.
This adds one entity delete
and one plugin call per create.
### How was this patch tested?
New and changed cases in the three hook suites:
| Test | `main` | Fix |
|---|---|---|
|
`TestIcebergTableHookDispatcher#testCreateTableAfterFailedDropReconcileDoesNotAdoptStaleEntity`
| fail | pass |
|
`TestIcebergTableHookDispatcher#testStagedCreateCommitRemovesStaleEntityBeforeImport`
| fail | pass |
|
`TestIcebergTableHookDispatcher#testCreateTableFailsWhenStaleEntityCannotBeRemoved`
| fail | pass |
|
`TestIcebergViewHookDispatcher#testCreateViewRemovesStaleEntityBeforeImport` |
fail | pass |
|
`TestIcebergNamespaceHookDispatcher#testRegisterTableWithoutOverwriteRemovesStaleEntityBeforeImport`
| fail | pass |
|
`TestIcebergNamespaceHookDispatcher#testRegisterViewRemovesStaleEntityBeforeImport`
| fail | pass |
|
`TestIcebergNamespaceHookDispatcher#testRegisterViewThrowsWhenStaleEntityRemovalFails`
| fail | pass |
`testRegisterViewSkipsImportWhenEntityExists` is removed, because that
behavior is the bug.
`testRegisterTableSkipsWhenGravitinoExists` becomes
`testRegisterTableWithOverwriteKeepsExistingEntity`.
With the fix, `TestIcebergTableHookDispatcher` (21),
`TestIcebergViewHookDispatcher` (19) and
`TestIcebergNamespaceHookDispatcher` (15) pass, as do
`TestIcebergAsyncPurge`,
`TestIcebergTableOperationExecutor` and `TestRESTService`.
`./gradlew -PskipITs :iceberg:iceberg-rest-server:test --tests
'*TestIcebergTableHookDispatcher' --tests '*TestIcebergViewHookDispatcher'
--tests '*TestIcebergNamespaceHookDispatcher'`
--
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]
