mchades commented on code in PR #14280:
URL: https://github.com/apache/gravitino/pull/14280#discussion_r4398136943
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -1054,8 +1059,13 @@ private TableEntity updateColumnsIfNecessaryWhenLoad(
// stored columns since, e.g. renamed a column while keeping its id. So
the columns are matched
// again against the entity being updated, which is the latest stored one.
This narrows the
// load/alter race but does not close it: if this load read the catalog
before a concurrent
- // alter and the store after it, the catalog snapshot is the stale side.
Closing that needs
- // alterTable and loadTable to exclude each other, which is tracked
separately.
+ // alter and the store after it, the catalog snapshot is the stale side,
and a column that the
+ // alter renamed is stored as dropped plus added, so it loses its id and
its tags.
+ //
+ // A tree lock cannot close this, because tree locks are local to one
server. Closing it needs
+ // the alter to record in the store that it is in progress before it calls
the catalog, and
+ // the load to read the stored version and that mark before it reads the
catalog, then write
+ // only if both are unchanged. That needs a new EntityStore API and is
tracked in #14279.
Review Comment:
[P3] Please also state that load must skip the write while the in-progress
mark is set, as #14279 already specifies. If load observes an active mark after
the catalog rename but before alter writes the store, both the version and mark
can remain unchanged; the condition stated here would still allow replacing the
column ID and losing its tags. Explicitly require no active mark in addition to
the unchanged version and epoch.
--
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]
