nevzheng opened a new pull request, #14260:
URL: https://github.com/apache/gravitino/pull/14260
### What changes were proposed in this pull request?
- **`RESTService` installs the table, view and namespace hook dispatchers
whenever an entity store
exists** (auxiliary mode), not only when authorization is on. The rule is
in a new package-private
`shouldInstallHookDispatchers(auxMode, authorizationEnabled)`.
- **New `trackCreates` constructor flag** on the three hook dispatchers, set
to whether authorization
is on. It gates the create, register and staged-commit import and owner
steps. Drop and rename
reconcile, and the orphan-schema cleanup, always run.
- The existing constructors default to `trackCreates=true`, so behavior with
authorization on is
unchanged.
### Why are the changes needed?
**With authorization off, Iceberg REST drop and rename did not touch the
Gravitino entity.** The entity
bookkeeping lives in the hooks, and the hooks were installed only with
authorization on. The Gravitino
REST API deletes the entity either way. A table dropped and recreated
through Iceberg REST then came
back with the old id, tags and policies on its next import.
Create-time import stays gated: with a static config provider the Iceberg
REST catalog may have no
Gravitino catalog, and an unguarded import would break creates that work
today.
Fix: #14259
### Does this PR introduce _any_ user-facing change?
Yes. In auxiliary mode with authorization off, Iceberg REST drop and rename
now delete or move the
Gravitino entity. Behavior with authorization on is unchanged. The
standalone server has no entity
store and is unchanged.
### How was this patch tested?
-
`TestRESTService#testHookDispatchersAreInstalledWheneverAnEntityStoreExists`
-
`TestIcebergTableHookDispatcher#testDropTableWithAuthorizationOffDeletesEntity`,
`#testRenameTableWithAuthorizationOffMovesEntity`,
`#testCreateTableWithAuthorizationOffSkipsImportAndOwnership`
-
`TestIcebergViewHookDispatcher#testDropViewWithAuthorizationOffRemovesFromEntityStore`
- **On `main`:** these tests do not compile, because the predicate and the
new constructors do not
exist. The bug is in the wiring in `RESTService#serviceInit`, which builds
the dispatcher chain inline
and is not reachable from a unit test.
- **With the fix:** all pass (`TestIcebergTableHookDispatcher` 21,
`TestIcebergViewHookDispatcher` 19,
`TestIcebergNamespaceHookDispatcher` 14, `TestRESTService` 3), as do
`TestIcebergAsyncPurge` and
`TestIcebergTableOperationExecutor`.
`./gradlew -PskipITs :iceberg:iceberg-rest-server:test --tests
'*TestIcebergTableHookDispatcher' --tests '*TestIcebergViewHookDispatcher'
--tests '*TestIcebergNamespaceHookDispatcher' --tests
'org.apache.gravitino.iceberg.TestRESTService'`
A follow-up can add an end-to-end check in auxiliary mode with authorization
off: load a table through
the Gravitino REST API, tag it, drop and recreate it through Iceberg REST,
and check that the new entity
has no tag.
--
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]
