yuqi1129 commented on code in PR #12350:
URL: https://github.com/apache/gravitino/pull/12350#discussion_r3712341877
##########
core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java:
##########
@@ -138,7 +138,7 @@ public static MetalakePO updateMetalakePOWithVersion(
MetalakePO oldMetalakePO, BaseMetalake newMetalake) {
Long lastVersion = oldMetalakePO.getLastVersion();
// Will set the version to the last version + 1 when having some fields
need be multiple version
Review Comment:
Updated the version comments to describe the OCC token explicitly. The
converters now advance from currentVersion, and parent fences keep
current_version and last_version aligned.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java:
##########
@@ -420,6 +425,22 @@ public boolean deleteMetalake(NameIdentifier ident,
boolean cascade) {
return true;
}
+ private void deleteMetalakeWithVersion(
+ NameIdentifier identifier, Long metalakeId, Long currentVersion) {
+ int deleted =
+ SessionUtils.getWithoutCommit(
+ MetalakeMetaMapper.class,
+ mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId,
currentVersion));
+ if (deleted == 0) {
+ throw optimisticLockException(identifier);
Review Comment:
Added TestMetalakeMetaService.testDeleteReportsOptimisticLockConflict. It
advances the row with a competing update, invokes the service
conditional-delete path with the stale version, and verifies
OptimisticLockException plus row preservation.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java:
##########
@@ -229,15 +230,19 @@ public <E extends Entity & HasIdentifier> CatalogEntity
updateCatalog(
AtomicInteger updateResult = new AtomicInteger(0);
try {
SessionUtils.doMultipleWithCommit(
- () ->
- updateResult.set(
- SessionUtils.getWithoutCommit(
- CatalogMetaMapper.class,
- mapper ->
- mapper.updateCatalogMeta(
- POConverters.updateCatalogPOWithVersion(
- oldCatalogPO, newEntity,
oldCatalogPO.getMetalakeId()),
- oldCatalogPO))),
+ () -> {
+ updateResult.set(
+ SessionUtils.getWithoutCommit(
+ CatalogMetaMapper.class,
+ mapper ->
+ mapper.updateCatalogMeta(
+ POConverters.updateCatalogPOWithVersion(
+ oldCatalogPO, newEntity,
oldCatalogPO.getMetalakeId()),
+ oldCatalogPO)));
+ if (updateResult.get() == 0) {
+ throw optimisticLockException(identifier);
+ }
+ },
Review Comment:
Added TestCatalogMetaService.testAlterReportsOptimisticLockConflict, which
forces a competing committed update and verifies the catalog service reports
OptimisticLockException.
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java:
##########
@@ -230,15 +231,19 @@ public <E extends Entity & HasIdentifier> SchemaEntity
updateSchema(
AtomicInteger updateResult = new AtomicInteger(0);
try {
SessionUtils.doMultipleWithCommit(
- () ->
- updateResult.set(
- SessionUtils.getWithoutCommit(
- SchemaMetaMapper.class,
- mapper ->
- ops.updatePO(
- mapper,
-
POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity),
- oldSchemaPO))),
+ () -> {
+ updateResult.set(
+ SessionUtils.getWithoutCommit(
+ SchemaMetaMapper.class,
+ mapper ->
+ ops.updatePO(
+ mapper,
+
POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity),
+ oldSchemaPO)));
+ if (updateResult.get() == 0) {
+ throw optimisticLockException(identifier);
+ }
Review Comment:
Added TestSchemaMetaService.testAlterReportsOptimisticLockConflict, which
commits a competing mapper update during the service updater and verifies that
the service throws OptimisticLockException.
--
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]
