roryqi commented on code in PR #13771:
URL: https://github.com/apache/gravitino/pull/13771#discussion_r4259245669
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -177,16 +178,21 @@ public Table loadTable(NameIdentifier ident) throws
NoSuchTableException {
entityCombinedTable =
TreeLockUtils.doWithTreeLock(schemaIdent, LockType.WRITE, () ->
importTable(ident));
} catch (EntityAlreadyExistsException e) {
- // HA race: another Gravitino node concurrently imported this table.
Reload from the
- // entity store to pick up the entity stored by the winning node.
- LOG.info(
- "Table {} was concurrently imported by another node; reloading
from store.", ident);
+ // A create-only import can conflict either because another Gravitino
node won an HA race,
+ // or because the external identifier is already stored under another
Gravitino path.
+ // Reload to distinguish a successful concurrent import from an
identity conflict.
+ LOG.info("Table {} import conflicted with existing metadata; reloading
from store.", ident);
entityCombinedTable =
TreeLockUtils.doWithTreeLock(ident, LockType.READ, () ->
internalLoadTable(ident));
if (!entityCombinedTable.imported()) {
throw new UnsupportedOperationException(
- "Table managed by multiple catalogs. This may cause unexpected
issues such as privilege conflicts. "
- + "To resolve: Remove all catalogs managing this table, then
recreate one catalog to ensure single-catalog management.");
+ "Table "
+ + ident
+ + " cannot be imported because its name or external
identifier is already "
+ + "managed under a different Gravitino identifier. Gravitino
does not "
+ + "automatically reconcile external table renames or manage
the same table "
+ + "through multiple paths. Remove `gravitino.identifier`
from the external table, "
+ + "then retry so it can be imported with a new identifier.");
Review Comment:
Updated in 182b2fbf86. Removing only `gravitino.identifier` is unsafe here
because the no-ID load path can bind the external object to the conflicting
path registration. The schema, table, and view errors now explicitly tell
operators not to do that: they must first either restore the external metadata
to match its existing registration or have an administrator safely remove the
stale registration and its privileges, tags, and other relationships. Only
after that cleanup should they clear the external identifier and load again.
The Iceberg and Paimon documentation is aligned, and the dispatcher tests
verify the explicit guidance while retaining the existing assertions that the
rejected import does not change the registered entity.
I have intentionally not added automatic reconciliation or a fresh-ID
recovery implementation for this out-of-band rename scenario. Gravitino cannot
safely decide that the existing registration and its governance relationships
should be deleted, and the current public drop operations also affect the
physical object. A metadata-only unregister workflow would require a separate
API/design; this bounded fix keeps the identity conflict explicit and directs
the administrator to resolve it manually.
--
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]
