mehakmeet commented on issue #10713:
URL: https://github.com/apache/gravitino/issues/10713#issuecomment-4238752805
So, I went through the code, and have couple of pointers in handling of the
cases where setOwner can fail:
Internal Objects (Role, Tag, Policy, Catalog, Job, Metalake) - we should
send 500 and rollback, since Gravitino has full control over these. This is
straightforward, as `CatalogHookDispatcher` already does this today. We can
handle other similar internal objects the same way, rollback by deleting the
object and re-throwing the exception.
External Objects (Schema, Table, Fileset, Topic, Model) - we should send 200
with a warning. We can't rollback here because `dropSchema`/`dropTable` etc.
would destroy real resources in the external system (Hive databases, Iceberg
namespaces, etc.), the creation already succeeded there, and only the internal
ownership assignment failed.
My approach for surfacing this to the user:
Add a `warnings` field to `BaseResponse.java` with `@JsonInclude(NON_EMPTY)`
so it's only present in the JSON when there's actually a warning. On normal
success, the response looks exactly the same as today.
Introduce an `OwnerNotSetException` that carries the successfully created
entity. When `setOwner` fails in a `HookDispatcher` for an external object, we
throw this instead of silently swallowing.
The REST endpoint catches `OwnerNotSetException`, extracts the created
object, builds the normal success response, and populates the warnings field
with a message like: `"Failed to set owner for schema <name of schema>:
<reason>. Owner can be set manually by any ancestor owner."`
Now IRS operations are little tricky., but we can apply the same idea there,
since those endpoints return Iceberg protocol response types
(`CreateNamespaceResponse`, `LoadTableResponse`, etc.) from the Iceberg
library, not our `BaseResponse` -- we can't add a warnings field to the
response body without breaking protocol compliance. Instead, we surface the
warning via an `X-Gravitino-Warning` HTTP header. Standard Iceberg clients will
simply ignore the extra header.
This gives us three distinct outcomes with no ambiguity:
Full success → HTTP 200, normal response (no warnings field)
Partial success (external objects, setOwner failed) → HTTP 200, normal
response + warnings
Failure (internal objects, setOwner failed) → HTTP 500, object rolled back,
error response
No status code changes needed, since clients may not handle 206 etc. as a
success, and the user always knows what happened.
@roryqi what do you think?
--
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]
