Victory-ET commented on code in PR #10613:
URL: https://github.com/apache/gravitino/pull/10613#discussion_r3024574996
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/FilesetMetaService.java:
##########
@@ -215,22 +215,38 @@ public <E extends Entity & HasIdentifier> FilesetEntity
updateFileset(
FilesetPO newFilesetPO =
POConverters.updateFilesetPOWithVersion(oldFilesetPO, newEntity,
checkNeedUpdateVersion);
if (checkNeedUpdateVersion) {
- // These operations are guaranteed to be atomic by the transaction. If
version info is
- // inserted successfully and the uniqueness is guaranteed by
`fileset_id + version +
- // deleted_at`, it means that no other transaction has been inserted
(if a uniqueness
- // conflict occurs, the transaction will be rolled back), then we can
consider that the
- // fileset meta update is successful
- SessionUtils.doMultipleWithCommit(
- () ->
- SessionUtils.doWithoutCommit(
- FilesetVersionMapper.class,
- mapper ->
mapper.insertFilesetVersions(newFilesetPO.getFilesetVersionPOs())),
- () ->
- SessionUtils.doWithoutCommit(
- FilesetMetaMapper.class,
- mapper -> mapper.updateFilesetMeta(newFilesetPO,
oldFilesetPO)));
- // we set the updateResult to 1 to indicate that the update is
successful
- updateResult = 1;
+ // These operations are performed atomically within a single
transaction. The version
+ // insert is protected by a unique constraint on `fileset_id + version
+ deleted_at`. If
+ // the meta update affects 0 rows (concurrent modification), the
transaction is rolled
+ // back — including the version insert — and the update is treated as
a conflict.
+ int[] metaUpdateCountRef = new int[1];
+ try {
+ SessionUtils.doMultipleWithCommit(
+ () ->
+ SessionUtils.doWithoutCommit(
+ FilesetVersionMapper.class,
+ mapper ->
mapper.insertFilesetVersions(newFilesetPO.getFilesetVersionPOs())),
+ () -> {
+ metaUpdateCountRef[0] =
+ SessionUtils.getWithoutCommit(
+ FilesetMetaMapper.class,
+ mapper -> mapper.updateFilesetMeta(newFilesetPO,
oldFilesetPO));
+ if (metaUpdateCountRef[0] == 0) {
+ throw new RuntimeException("Failed to update the entity: " +
identifier);
+ }
+ });
+ updateResult = 1;
+ } catch (RuntimeException re) {
+ if (metaUpdateCountRef[0] == 0) {
+ // The meta update matched no rows; the transaction was rolled
back,
+ // including the version insert above.
+ updateResult = 0;
Review Comment:
thanks for pointing that out, that was uneccessary since we only ever set to
1 in the success path, and the exception is thrown in the case of zero
regardless. I've taken that out.
--
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]
