luoyuxia commented on issue #3483: URL: https://github.com/apache/fluss/issues/3483#issuecomment-4756444598
I looked through the current code, and I am not fully sure under what exact sequence case (2) can happen. For case (1), the race is clear: a late `commitLakeTableSnapshot` can still call `LakeTableHelper.registerLakeTableSnapshotV2(tableId, ...)`, and `registerLakeTableSnapshotV2` does an unconditional `upsertLakeTable` for that `tableId`. Since `processDropTable` only removes the table from the in-memory `LakeTableTieringManager` and the physical `/tabletservers/tables/<id>` cleanup happens later, a late commit can recreate `/tabletservers/tables/<oldId>/laketable`. For case (2), a normal in-flight commit from the old table does not obviously explain writing an old offsets path into `/tabletservers/tables/<newId>/laketable`: * `TieringCommitOperator.commitWriteResults` re-fetches `TableInfo` by `tablePath` before preparing/committing. If the table was dropped and recreated, `currentTableInfo.getTableId()` should differ from the table id carried by the old committable, and the commit is aborted. * `prepareLakeSnapshot(tableId, tablePath, ...)` stores the offsets file under the `tableId` passed in the request, and the client checks the response table id matches. * the final commit request uses the same `tableId` and the offsets path returned by that prepare call. So, on the ordinary path, an old-table commit should either write under `<oldId>` or fail after the recreate; I do not see how it would naturally become a `<newId>` commit while keeping a `<oldId>` offsets path. The only plausible path I can find from the current code is the "missing lake snapshot" recovery path in `TieringCommitOperator.checkFlussNotMissingLakeSnapshot`: 1. `db.t` is dropped and recreated, so Fluss now has `<newId>` for the same `TablePath`. 2. The lake catalog still exposes an old snapshot for `db.t`, and that snapshot property still contains `FLUSS_LAKE_SNAP_BUCKET_OFFSET_PROPERTY = .../lake/db/t-<oldId>/metadata/<uuid>.offsets`. 3. The new table's tiering round calls `lakeCommitter.getMissingLakeSnapshot(flussCurrentLakeSnapshot)` and gets that old lake snapshot as a missing snapshot. 4. `checkFlussNotMissingLakeSnapshot` then calls `flussTableLakeSnapshotCommitter.commit(tableId, missingSnapshotId, lakeSnapshotOffsetPath, ...)`, where `tableId` is the current Fluss table id (`<newId>`) but `lakeSnapshotOffsetPath` came from the old lake snapshot property (`<oldId>` path). 5. The coordinator currently does not validate that the offsets path belongs to the same `tableId` / table incarnation before writing `LakeSnapshotMetadata`, so it can persist that stale path under `/tabletservers/tables/<newId>/laketable`. But this requires the new table's lake committer to be able to see the old lake snapshot after `dropTable + createTable`. If the lake table is truly dropped/recreated together with the Fluss table, I do not yet see how step 2 would be true. It would help to clarify whether the lake table/data is intentionally retained across the Fluss drop/recreate, or whether the drop only removed Fluss metadata/remote offsets but left lake snapshots visible. Regardless, I think the robust server-side protection is still useful: when handling `commitLakeTableSnapshot`, reject commits for table ids that no longer exist / are queued for deletion, and validate that the incoming `tiered_offsets` path matches the expected table id/table path before upserting `/laketable`. -- 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]
