yuqi1129 commented on code in PR #12551:
URL: https://github.com/apache/gravitino/pull/12551#discussion_r3851263023
##########
core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java:
##########
@@ -215,24 +219,35 @@ public <E extends Entity & HasIdentifier> TableEntity
updateTable(
oldTablePO.getMetalakeId());
}
},
- () ->
- updateResult.set(
- SessionUtils.getWithoutCommit(
- TableMetaMapper.class,
- mapper -> ops.updatePO(mapper, newTablePO, oldTablePO))),
- () ->
- SessionUtils.doWithoutCommit(
- TableVersionMapper.class,
- mapper -> {
- mapper.softDeleteTableVersionByTableIdAndVersion(
- oldTablePO.getTableId(),
oldTablePO.getCurrentVersion());
- mapper.insertTableVersionOnDuplicateKeyUpdate(newTablePO);
- }),
() -> {
- if (updateResult.get() > 0) {
- TableColumnMetaService.getInstance()
- .updateColumnPOsFromTableDiff(oldTableEntity,
newTableEntity, newTablePO);
+ // This update is the decision point for the whole transaction.
current_version is the
+ // table's OCC token: if another writer changed the table after we
read it, that writer
+ // has already increased the token and this UPDATE changes zero
rows. Throwing here
+ // rolls back the transaction before it can touch the version
history or columns.
+ int updated =
+ SessionUtils.getWithoutCommit(
+ TableMetaMapper.class, mapper -> ops.updatePO(mapper,
newTablePO, oldTablePO));
+ if (updated == 0) {
+ throw tableWriteFailure(identifier, oldTablePO);
}
+ },
+ () -> {
+ // The table details live in table_version_info, while table_meta
points to the current
+ // version. These two rows must move together. This step runs only
after the table_meta
+ // CAS above succeeds, so a losing writer cannot overwrite the
winner's version row.
Review Comment:
The row in table_version_info is keyed by (table_id, version) and this
upsert has no version guard of its own, so two writers racing to produce
version N+1 would target the same key and the later one would silently win.
What prevents that is the ordering: the loser's table_meta CAS above matches no
row and rolls the transaction back before it ever reaches this statement.
The following is an example:
A row in `table_meta` with key(id, current_version) is (12, 2),
`table_version_info` with key(id, version) is (12, 2). Two threads concurrently
update the table; they will first update the table `table_meta` to (12, 3),
then use CAS to update `table_version_info`. If the first step fails, the code
will not go into the second one.
> so a losing writer cannot overwrite the winner's version row.
Because altering the table `table_version_info` is an upsert (create or
update when conflicts), the final result will always carry the information of
the last operation.
--
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]
