mchades commented on code in PR #13771:
URL: https://github.com/apache/gravitino/pull/13771#discussion_r4259010610
##########
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:
**[P2] Include stale-registration cleanup in the recovery message**
Suppose `t1` (ID A) and `t2` (ID B) are both registered in the same
Gravitino schema. Delete `t2` and rename `t1` to `t2` **only in HMS**,
preserving A in the renamed table's properties and leaving both Gravitino
registrations untouched. Loading `t2` now reaches this conflict.
Following this message and only removing `gravitino.identifier` does not
import a fresh ID: the [no-ID branch of
`internalLoadTable`](https://github.com/apache/gravitino/blob/782c1e2e0ae60f2e1ca99d68dd67cef4b07ef9a7/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java#L643-L659)
finds the existing `t2` registration, reuses B and sets `imported=true`. The
renamed table consequently inherits B's privileges and tag relationships.
Please align this message and the equivalent schema/view messages with the
updated documentation: safely clean up stale Gravitino registrations and their
relationships before clearing the external identifier and re-importing. Add a
regression for this recovery sequence that verifies a fresh ID is assigned and
B's relationships are not inherited.
--
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]
