luoyuxia commented on PR #3494: URL: https://github.com/apache/fluss/pull/3494#issuecomment-4882290153
> > @Jackeyzhe Thanks for working on this. > > I wonder if we can make this fix simpler and close the race at the ZK write boundary instead. The current approach adds coordinator-side lifecycle checks before submitting the actual ZK write to `ioExecutor`, but that check can become stale before `registerLakeTableSnapshotV2` runs. For example, the commit event may pass the context check while the table still exists, then `dropTable` can delete `/tabletservers/tables/<tableId>`, and the later IO task can still recreate `/tabletservers/tables/<tableId>/laketable` because `upsertLakeTable` uses `creatingParentsIfNeeded()`. > > Would it be enough to change the create branch in `ZooKeeperClient.upsertLakeTable` to avoid recreating the table parent? > > ```java > > // Do not recreate /tabletservers/tables/<tableId> for a late commit. > > zkClient.create().forPath(zkPath, zkData); > > ``` > > > > > > > > > > > > > > > > > > > > > > > > Then a late commit after `deleteTableAssignment(tableId)` would fail with `NoNodeException` instead of resurrecting the old tableId znode. We can convert that to `TableNotExistException` for the commit response. The coordinator-side queued/deleted check can still be kept as an early rejection, but the ZK write itself should be what prevents the resurrection race. > > Thanks for pointing this out. I agree that the coordinator-side check is only an early rejection and does not fully close the race before the actual ZK write. > > I'll update the fix to make the ZK write boundary authoritative: for the create branch in ZooKeeperClient.upsertLakeTable, avoid creating missing parents and use create().forPath(...) directly. Then a late commit after the table assignment znode has been deleted will fail with NoNodeException instead of recreating /tabletservers/tables/. I'll convert that failure to TableNotExistException for the commit response. > > I'll keep the coordinator-side queued/deleted check as an early rejection, but rely on the ZK create semantics to prevent resurrection. Can we remove the coordinator-side queued/deleted check to keep code simple? -- 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]
