yuqi1129 opened a new issue, #12639:
URL: https://github.com/apache/gravitino/issues/12639

   ### What would you like to be improved?
   
   The optimistic-concurrency-control (OCC) work landed service by service 
(metalake -> catalog -> schema -> table), and each PR copied the previous 
service's helpers instead of extracting shared ones. Follow-up to 
https://github.com/apache/gravitino/pull/12551#discussion_r3853142345.
   
   Four patterns are now duplicated across 
`core/src/main/java/org/apache/gravitino/storage/relational/service/`:
   
   1. **Write-failure classification** — lock-and-reread by id 
(`select*MetaByIdForUpdate`), compare natural-key fields, then return 
`NoSuchEntityException` or `ExceptionUtils.concurrentModification`:
      - `MetalakeMetaService.metalakeWriteFailure` 
(`MetalakeMetaService.java:408`)
      - `CatalogMetaService.catalogWriteFailure` (`CatalogMetaService.java:482`)
      - `SchemaMetaService.schemaWriteFailure` (`SchemaMetaService.java:538`)
      - `TableMetaService.tableWriteFailure` (in PR #12551)
   
      These already drifted: the schema version re-maps the PO 
(`physicalToLogicalSchemaPO`) and uses a `noSuchSchemaException` helper, while 
the catalog/metalake versions inline the exception and fold the null check into 
one condition. A future change to the classification rules (an extra field to 
compare, different lock semantics) has to be made identically in four places.
   
   2. **Single-row CAS delete** — soft-delete by `(id, currentVersion)`, and on 
`deleted == 0` throw the write failure above: `MetalakeMetaService.java:398`, 
`CatalogMetaService.java:437`, `SchemaMetaService.java:406`.
   
   3. **Cascade child delete** — batch soft-delete children with versions and 
throw `ExceptionUtils.concurrentChildModification` when the affected-row count 
is short: `CatalogMetaService.java:507`, `MetalakeMetaService.java:428` and 
`:457`, `SchemaMetaService.java:572`.
   
   4. **Parent-row lock before a child write** — share/exclusive lock on the 
parent id, re-compare the parent name to catch a rename, throw 
`NoSuchEntityException`: `CatalogMetaService.lockMetalakeForCatalogCreate:462`, 
`SchemaMetaService.lockCatalogForSchemaCreate:466` / 
`lockCatalogForSchemaDelete:491`, 
`ModelVersionMetaService.lockSchemaForModelVersionWrite:449`.
   
   ### How should we improve?
   
   Extract one set of generic helpers (e.g. in a new `OccWriteSupport`, or 
extending `ExceptionUtils`) that all services call, roughly:
   
   - `RuntimeException writeFailure(NameIdentifier ident, EntityType type, T 
observed, Function<Long, T> lockingLookup, BiPredicate<T, T> sameIdentity)`
   - `void deleteWithVersion(NameIdentifier ident, IntSupplier softDelete, 
Supplier<RuntimeException> onMiss)`
   - `void deleteChildrenWithVersions(NameIdentifier parentIdent, EntityType 
childType, EntityType parentType, List<T> children, ToIntFunction<List<T>> 
softDelete)`
   - `void lockParentForChildWrite(...)`
   
   Then migrate metalake / catalog / schema / table (and model-version) to the 
shared helpers, keeping the existing behaviour and tests. Doing this in one 
follow-up avoids a fifth copy when OCC is extended to the remaining entity 
types (fileset, topic, model, ...).
   


-- 
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]

Reply via email to