yuqi1129 opened a new pull request, #13633: URL: https://github.com/apache/gravitino/pull/13633
### What changes were proposed in this pull request? The P0 part of #13621: state the `EntityStore` contract that callers already rely on, and make the implementation and call sites match it. No method is added. **Concurrency and error contract (#13622)** - Class Javadoc on `EntityStore`: every write is atomic including owned data; concurrent writes to one entity are resolved optimistically and the loser fails with `OptimisticLockException` and changes nothing; the store never retries; an outcome table for missing entity, taken name, lost write, non-empty delete, unsupported type and storage failure; reads may come from the per-server cache. - `update`: the updater is called exactly once with the stored (not cached) entity, must not have outside effects, may rename but must not change the id. **Documentation and behavior aligned (#13623)** - `put(e)` was documented as overwrite but inserts strictly; the Javadoc now says so. - `delete` reports a missing entity with `false`. The redundant `NoSuchEntityException` handling around it is removed from `ManagedTableOperations`, `ManagedSchemaOperations`, `ManagedFunctionOperations`, `ManagedSemanticModelOperations` and `IcebergTableHookDispatcher`. The external-catalog dispatchers keep theirs for now; they are rewritten by #13625. - `batchGet` is documented as it behaves: missing entities are left out, the order is unspecified, and a read failure is thrown. `InMemoryEntityStore` no longer returns `null` entries. - `cascade` is documented per entity type: it applies to metalakes, catalogs and schemas (listing which children block a non-cascade delete) and has no effect elsewhere. - `list(namespace, class, type)` defaulted to `allFields = true` while `RelationalEntityStore` passes `false`; the default now matches the only implementation. No caller lists users or groups, the only types the flag affects, through this overload. - `RelationalEntityStore.update` also invalidates the new name after a rename, so a stale cache entry for that name cannot hide the renamed entity. - `batchPut`, `batchDelete` and the model-version `overwrite` behavior are documented. **`executeInTransaction` (#13624)** - Deprecated and turned into a default method that throws `UnsupportedOperationException`. `RelationalEntityStore` never implemented it and nothing calls it; each write method is atomic on its own. Multi-entity atomicity is tracked in #13632. It is deprecated rather than removed so that implementations that override it still compile. ### Why are the changes needed? Callers and implementers could only reverse-engineer `RelationalEntityStore`: the Javadoc omitted the conflict behavior everyone depends on and contradicted the implementation in several places. The capability work in #13621 builds on this contract. Fix: #13622 Fix: #13623 Fix: #13624 ### Does this PR introduce _any_ user-facing change? No REST or configuration change. For `EntityStore` implementers, `executeInTransaction` is deprecated and no longer abstract. ### How was this patch tested? - New `TestEntityStoreContract` runs the documented outcomes against `RelationalEntityStore` on H2, MySQL and PostgreSQL (36 cases): strict put, updater called once, updater failure writes nothing, id change rejected, rename to a taken name, update of a missing entity, a concurrent write winning with `OptimisticLockException`, delete of a missing entity, non-cascade delete of a non-empty schema, `batchGet` leaving out missing entities, the renamed entity not hidden by a stale cache entry, and `executeInTransaction` being unsupported. - New `TestEntityStoreDefaults` for the delegation of the default methods. - Updated `TestManagedSemanticModelOperations` and `TestIcebergTableHookDispatcher` to the documented `delete` result, and `TestRelationalEntityStore` so its mocked backend returns the updated entity instead of `null`. - Ran the tests of the changed classes: `TestEntityStoreContract`, `TestEntityStoreDefaults`, `TestMemoryEntityStore`, `TestRelationalEntityStore*`, `TestManaged*Operations*`, `TestIcebergTableHookDispatcher`, `TestLanceConcurrentRepairStress`, and `:core:javadoc`. -- 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]
