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

   ### What would you like to be improved?
   
   #12782 added `OccWriteSupport.updateWithVersion(IntSupplier, 
Supplier<RuntimeException>)`, which centralises the "issue the UPDATE, check 
affected-rows == 0, classify the miss via a re-lookup" idiom, and 
`PolicyMetaService` now uses it.
   
   Four services still hand-roll the same idiom inline:
   
   - `TableMetaService.java:241`
   - `TopicMetaService.java:135`
   - `CatalogMetaService.java:253`
   - `ModelVersionMetaService.java:392` and `:243`
   
   Both forms are semantically identical today, so this is not a bug. The cost 
is that a future fix to the CAS-miss classification or retry behaviour applied 
only to `OccWriteSupport.updateWithVersion` would silently miss those call 
sites, reintroducing the class of inconsistency the OCC work set out to remove.
   
   A smaller, related duplication: the "look up the metalake by name, throw 
`NoSuchEntityException` if it is missing" block is repeated verbatim in 
`PolicyMetaService.insertPolicy` and `CatalogMetaService.insertCatalog`.
   
   ### How should we improve?
   
   - Migrate the four services to `OccWriteSupport.updateWithVersion`, one 
service per commit so each is independently reviewable and revertible.
   - Extract a shared `requireMetalakeByName(String)` returning a validated 
`MetalakePO`, and use it from both insert paths.
   
   Each migrated service needs its existing storage tests re-run against H2, 
MySQL and PostgreSQL, since the CAS behaviour is dialect-sensitive.
   
   Follow-up from the review of #12782.
   


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