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.\n\nI 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 t
 he 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]

Reply via email to