justinmclean opened a new issue, #10656:
URL: https://github.com/apache/gravitino/issues/10656
### What would you like to be improved?
SchemaOperationDispatcher#createSchema creates the schema in the underlying
catalog and then persists the corresponding SchemaEntity to the Gravitino
store. If store.put(schemaEntity, true) throws, the exception is caught,
logged, and the method still returns a successful schema result.
This causes a false-success outcome:
The REST API returns success for the create request.
- Success events are emitted.
- The schema may exist in the external catalog, but Gravitino metadata was
not persisted.
Although a later loadSchema may lazily import the missing metadata, that is
only a best-effort repair after the fact. The original create operation has
already reported success despite an internal persistence failure, which can
lead to inconsistent behavior across audit/authorization/imported metadata.
/Users/justinmclean/gravitino/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java:157
Possible solution
Fail the create operation when store.put(...) fails instead of swallowing
the exception.
Options:
Propagate a runtime exception after logging so the API returns failure.
If partial creation in the underlying catalog must be avoided, attempt
rollback of the external schema creation before failing.
Add a test covering entity-store failure during schema creation and
verifying the request does not return success.
### How should we improve?
See
gravitino/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java:157
Fail the create operation when store.put(...) fails instead of swallowing
the exception.
Options:
- Propagate a runtime exception after logging so the API returns failure.
- If partial creation in the underlying catalog must be avoided, attempt
rollback of the external schema creation before failing.
Here's a unit test to help:
```
@Test
public void testCreateSchemaShouldFailWhenStorePutFails() throws
IOException {
Namespace ns = Namespace.of(metalake, catalog);
NameIdentifier schemaIdent = NameIdentifier.of(ns, "schema2");
Map<String, String> props = ImmutableMap.of("k1", "v1", "k2", "v2");
doThrow(new IOException("store put
failed")).when(entityStore).put(any(), anyBoolean());
Assertions.assertThrows(
RuntimeException.class, () -> dispatcher.createSchema(schemaIdent,
"comment", props));
Assertions.assertFalse(entityStore.exists(schemaIdent, SCHEMA));
Assertions.assertThrows(
NoSuchEntityException.class,
() -> entityStore.get(schemaIdent, SCHEMA, SchemaEntity.class));
}
```
--
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]
