yuqi1129 opened a new issue, #12597:
URL: https://github.com/apache/gravitino/issues/12597
### What would you like to be improved?
Once an entity type gets optimistic concurrency control, its drop path can
fail in a way the caller cannot recover from by retrying.
`TableOperationDispatcher.dropTable` and `purgeTable`,
`SchemaOperationDispatcher.dropSchema` and `ViewOperationDispatcher.dropView`
all have the same shape:
```java
if (droppedFromCatalog) {
try {
store.delete(ident, <TYPE>);
} catch (NoSuchEntityException e) {
LOG.warn(...);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
```
The external object is dropped first, and the stored row is removed only
when that succeeded. If `store.delete` now loses its version compare-and-set
because another node altered the entity in between, the request fails. A retry
of the same call finds the external object already gone —
`HiveCatalogOperations.dropTable` returns `false` in that case — so
`droppedFromCatalog` is `false`, the block is skipped, and the stored row is
never removed. This block is the only place that removes it, and the existing
cleanup jobs (`OrphanedSchemaCleanup`, `SchemaEntityCleaner`) only collect
schemas, so nothing else will.
Current state per entity type:
| Path | CAS on delete today | What the caller sees
|
|----------------------------|---------------------|---------------------------------------------------------------------------------------------------------|
| `dropSchema` | yes (#12456) | `RuntimeException` — no
`OptimisticLockException` catch, so a conflict is reported as a generic failure
|
| `dropTable` / `purgeTable` | yes (#12551) |
`OptimisticLockException`
|
| `dropView` | not yet | will inherit this when
view OCC lands
|
| `dropTopic` | not yet | different shape:
`store.delete` is not gated on `droppedFromCatalog`, so a retry does re-attempt
it |
So the behavior is already inconsistent between schema and table, and
fileset, model, view and topic will each meet the same decision as the OCC
series continues.
### How should we improve?
Decide this once for every entity type rather than per PR. The options we
see:
1. **Check identity instead of version when deleting.** A drop is not a
read-modify-write, and delete is idempotent: a conflict only says the row moved
on. Keeping the identity check (renamed or moved away still reports not found)
while dropping the version predicate removes the failure entirely.
2. **Absorb the conflict with a bounded retry**, placed somewhere all drop
paths share rather than in one dispatcher.
3. **Keep failing, and add cleanup** for orphaned table and view rows, the
way schemas already have.
Whichever is chosen, the exception type surfaced should be the same for all
entity types.
Found while reviewing #12551. Related: #12456, #12345.
--
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]
