yuqi1129 commented on issue #14420: URL: https://github.com/apache/gravitino/issues/14420#issuecomment-6600966241
After examining #14557, I think we should distinguish a bounded fix for stale owner assignment from a complete consistency solution. **Cause and background** The owner hook is synchronous, not an asynchronous task. However, `TableOperationDispatcher.createTable` finishes its protected create/registration work before `TableHookDispatcher` assigns the creator as owner. Another request can drop or replace the table between these steps. A name-only lookup can then produce the reported 404 after creation has completed, or assign ownership to a same-name replacement. This relates to the identity, external-operation ordering and authorization-callback concerns tracked in #13730 and #13733. It should not be attributed solely to TreeLock removal: the gap between the create and ownership steps exists independently, and a local lock would not establish cross-server consistency. The previously reported stress-test pass is useful evidence, but does not by itself establish an ordering guarantee; this comment does not claim a new reproduction on the latest main. **Why the current proposal needs more consideration** #14557 provides useful protection: check the ID returned by creation and insert an owner only if that exact table is still present and has no owner, with the checks and write serialized in the database. That can prevent a stale request from changing the stored owner of a replacement and skip initialization after deletion. It does not make table creation and ownership initialization atomic. The table can still be visible before its owner is initialized; authorization-plugin notifications also remain outside the database transaction and use names. Meanwhile, adding `supportsInitialTableOwner`/`setInitialTableOwner` across `OwnerDispatcher`, `SupportsRelationOperations` and their implementation layers expands core contracts for a specific lifecycle operation. Defaulting the capability to false also means existing custom implementations can still compile but reject authorized table creation. We do not yet have a complete, validated solution with an acceptable interface and compatibility cost. This does not mean the individual failure modes cannot be improved, but the current proposal should not be treated as a complete fix or merged solely on that basis. **Possible evolution** 1. Define the local invariant first: where the table registration and owner relation live in the same database, aim to commit them in one transaction. This applies to the local metadata portion of both managed tables and external-table registrations. The existing relational `EntityStore.executeInTransaction` entry point is currently unsupported, so this requires deliberate storage-boundary work, not simply wrapping the hook. 2. Keep external effects separate. Even a managed catalog may create physical data outside that database. External catalog creation, physical storage changes and authorization-plugin calls cannot be rolled back by a local transaction. Strict cross-system atomicity would require participating systems to support a coordination protocol; introducing distributed transactions is not proposed here. 3. For those external steps, investigate explicit partial-completion states, stable operation/entity IDs, idempotent recovery and reliable post-commit notifications. Recovery must not blindly replay creation or use a name alone to modify a replacement object. 4. If a smaller interim patch is practical, limit its claim to identity-fenced, insert-if-absent ownership initialization and the misleading post-create 404. Preserve real principal/storage failures and existing cache invalidation/notifications, minimize core API expansion, and add deterministic drop/recreate tests. Merely catching 404 or checking the ID before a separate name-based write is insufficient. Until such a design is implemented and validated, the remaining visibility window, partial-success behavior and notification limitations should be stated explicitly in code comments/documentation. Passing a stress test or protecting the stored owner relation alone should not be presented as full create/ownership atomicity. -- 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]
