luoyuxia commented on issue #3483:
URL: https://github.com/apache/fluss/issues/3483#issuecomment-4756341073
After digging into current `main`: **#1 (resurrect) is real but
low-probability; #2 (leak old id into the new pointer) shouldn't be possible on
the current code.**
**Version.** This is reported on `main` / `fluss-flink-2.2-1.0-SNAPSHOT`,
which already contains #2316 ("fix potential dirty commit on re-create table",
2026-01-07). That guard matters below.
### Why #2 (new id's `/laketable` pointing at `<table>-<oldId>/…offsets`)
shouldn't happen
A tiering commit carries a **single** tableId end to end, and the offsets
path is derived from that same id — so the id written into the znode and the id
embedded in the path are always the same value:
- `TieringCommitOperator` takes `tableId = tableBucket.getTableId()`
(`TieringCommitOperator.java:146`) and uses that one id for the whole collect →
check → prepare → commit flow; it never re-resolves the id mid-flight.
- The offsets path is generated during *prepare* from
`tableBucketOffsets.getTableId()`
(`CoordinatorService.prepareLakeTableSnapshot` →
`LakeTableHelper.storeLakeTableOffsetsFile:186` →
`FlussPaths.remoteLakeTableSnapshotOffsetPath`, i.e.
`lake/{db}/{table}-{tableId}/metadata/{uuid}.offsets`).
- On *commit*, the znode is keyed by the commit map's key
(`handleCommitLakeTableSnapshotV2` → `entry.getKey()` →
`registerLakeTableSnapshotV2(tableId, …)` → `upsertLakeTable(tableId, …)`), and
that key is the same committable tableId.
Since both the path's `-<id>` segment and the znode's id come from the
*same* `tableId`, a commit can never write an `oldId` path under a `newId`
znode. On top of that, #2316's check at `TieringCommitOperator:215` aborts the
commit outright when `getTableInfo(tablePath).getTableId() != tableId`, so a
committable prepared under `oldId` is never even submitted once the table has
been recreated as `newId`.
So the observed `/tables/62/laketable → timeseries-58/…offsets` is almost
certainly **stale state left over from dev-loop cycles before #2316**, not
something the current code can newly produce (the report itself notes it
accumulated "after dev-loop drop+recreate cycles").
### Why #1 (resurrecting `/tables/<oldId>/laketable`) is still possible
(low-probability)
#2316's check is a TOCTOU snapshot. There's a window between the client's
`getTableInfo` check and the commit actually landing on the coordinator (build
LakeCommitter → write lake → `prepareLakeTableSnapshot` →
`commitLakeTableSnapshot`):
```
client: getTableInfo() → currentId == oldId, check passes (table still
alive)
…write lake / prepare… (table gets
dropped here)
server: commitLakeTableSnapshot(oldId) → registerLakeTableSnapshotV2
upserts unconditionally
→ /tables/<oldId>/laketable recreated (id
self-consistent, but table is gone)
```
Here both the znode id and the path id are `oldId` (self-consistent) — it
just resurrects a znode for an already-dropped table. The client check can't
cover this because the drop happens *after* the check, on the server side.
### Suggested fix (narrowed)
Given the above, only #1 needs a server-side guard; the path↔id validation
(original suggestion #2) is no longer necessary post-#2316. In
`handleCommitLakeTableSnapshotV2` (and V1), before
`registerLakeTableSnapshotV2`, reject the commit if the tableId is no longer
live / is queued for deletion in `coordinatorContext`. Because the coordinator
processes events single-threaded, the drop event removes the tableId from
`coordinatorContext` before any later commit event is handled, so this check is
race-free without extra locking.
cc @Jackeyzhe — happy to be proven wrong if you can construct a #2 repro on
current `main`.
--
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]
