This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 735918739d [#14279] docs(core): Document the load/alter column race
that can drop column tags (#14280)
735918739d is described below
commit 735918739d56a26b7ec7d73939ba38441eb66a8e
Author: Qi Yu <[email protected]>
AuthorDate: Wed Oct 28 19:34:02 2026 +0800
[#14279] docs(core): Document the load/alter column race that can drop
column tags (#14280)
### What changes were proposed in this pull request?
Document the load/alter column race; the code is not changed:
- `TableOperationDispatcher`: comments in `alterTable` and
`updateColumnsIfNecessaryWhenLoad` explain why the race is still open
across servers and what the full fix needs.
- `docs/manage-tags-in-gravitino.md`: a note that a column rename that
runs concurrently with a load or alter of the same external-backed table
can drop the column's tags.
### Why are the changes needed?
A fix that only takes a stronger tree lock would close the race on one
server but not across servers, so it is not worth shipping on its own.
The full fix needs a new EntityStore API (#13621) and is tracked in
#14279.
Part of #14279
### Does this PR introduce _any_ user-facing change?
Documentation only.
### How was this patch tested?
Comments and docs only. `./gradlew :core:spotlessCheck` passes.
---
.../gravitino/catalog/TableOperationDispatcher.java | 16 ++++++++++++++--
docs/manage-tags-in-gravitino.md | 8 ++++++++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
index 507b3c76c5..4e50f44aa4 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
@@ -280,6 +280,11 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
// at the same time.
// 2. if the TableChange.RenameTable only change the table name, we need
to acquire write lock
// to on the schema.
+ //
+ // The read lock on the table does not order this alter against a
concurrent load or another
+ // alter of the same table, and no tree lock orders them across servers.
Either of them can
+ // write a stale column list over this alter and turn a column rename into
a drop plus an add,
+ // which removes the renamed column's tags. See
updateColumnsIfNecessaryWhenLoad and #14279.
NameIdentifier nameIdentifierForLock = ident;
String schemaName = ident.namespace().level(2);
Arrays.stream(changes).forEach(TableOperationDispatcher::validateColumnChange);
@@ -1096,8 +1101,15 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
// 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, operation epoch and in-progress
mark before it reads
+ // the catalog, skipping the store write while the mark is set. The load
may write only if the
+ // version and epoch are unchanged and no in-progress mark is set. That
needs a new EntityStore
+ // API and is tracked in #14279.
Table tableFromCatalog = combinedTable.tableFromCatalog();
return TreeLockUtils.doWithTreeLock(
tableIdent,
diff --git a/docs/manage-tags-in-gravitino.md b/docs/manage-tags-in-gravitino.md
index e99d8c5196..1d23f832c2 100644
--- a/docs/manage-tags-in-gravitino.md
+++ b/docs/manage-tags-in-gravitino.md
@@ -292,6 +292,14 @@ raw_events.supports_tags().associate_tags(["pii"], None)
</TabItem>
</Tabs>
+:::note
+Column tags follow the column's internal id. For tables whose metadata is kept
by an external
+catalog, such as Hive, Iceberg or JDBC catalogs, a column rename that runs
concurrently with a load
+or another alter of the same table, on any Gravitino server, can be stored as
a dropped column plus
+a new one. The renamed column then loses its tags. Avoid loading or altering a
table while one of
+its columns is being renamed, and check the column's tags after the rename.
+:::
+
### Assign and Remove Tag Values
Tag values are updated as pairs of tag name and value. Adding a pair preserves
the tag's other
