[DISCUSS] getInitialTable table cache: Hive and Glue sync clients disagree on whether a cached read may hide the client's own write

[wombatu-kun]

Problem

HoodieHiveSyncClient and AWSGlueCatalogSyncClient both memoize the metastore table in a per-instance initialTableByName map, filled lazily by getInitialTable. It was added in HUDI-9365 ("Reduce overhead of Hive and AWS Glue sync tools") to avoid repeated getTable calls during a sync. In both clients exactly three reads go through it: getLastCommitTimeSynced, getLastCommitCompletionTimeSynced and getTableLocation.

Neither client invalidates that entry when it mutates the table. In HoodieHiveSyncClient eight methods call alter_table or dropTable (updateTableProperties, updateSerdeProperties, createOrReplaceTable, updateLastReplicatedTimeStamp, deleteLastReplicatedTimeStamp, updateLastCommitTimeSynced, updateHoodieWriterVersion, dropTable) and none of them touch the map. So within a single sync run the client can hand back a last_commit_time_sync or a table location that it has itself already overwritten.

The two clients then diverge, by accident rather than by design. AWSGlueCatalogSyncClient.tableExists performs a real GetTable and re-puts the result into initialTableByName, and HiveSyncTool.syncHoodieTable calls tableExists as its very first step, so on Glue the entry is refreshed immediately before every cached read. HoodieHiveSyncClient.tableExists returns the metastore client's boolean and never populates the map, so on Hive the entry survives from the first getTableLocation of the run until the client is closed.

Why it is latent today rather than live

The one path that exercised the divergence was HiveSyncTool.doSync() under the default ALL strategy with hoodie.datasource.hive_sync.skip_ro_suffix=true. initTableNameVars makes roTableName the bare table name, so the same metastore table was synced twice in one run, first as read-optimized and then again by the "sync origin table" step. On Hive the second call read the pre-write snapshot, isAlreadySynced returned false, and updateSerdeProperties rewrote the input format to HoodieParquetRealtimeInputFormat; the refreshed entry on Glue would have made the same sequence short-circuit instead. That is what #16637 and #12011 report, and the open PR #19427 fixes it by removing the redundant second sync.

With #19427 in, I could not find any remaining path in the Hive sync code that reads a cached field after writing it. GlobalHiveSyncTool reads replication timestamps live rather than through getInitialTable, recreateAndSyncHiveTable never re-reads after replacing the table, and every other entry point builds a fresh HiveSyncTool per sync. So this is currently an inconsistency between two implementations of the same interface rather than a reproducible failure.

The design question

Which contract is getInitialTable meant to have? The two answers imply opposite patches, and I do not think the code settles it.

1. A per-run snapshot, as the name suggests: the table as it was when this sync started. Under this reading isAlreadySynced is asking "was this table already up to date before I began", which is a reasonable question, HoodieHiveSyncClient is correct as written, and the defect is AWSGlueCatalogSyncClient.tableExists quietly breaking the snapshot on every call.
2. A cache of current state. Under this reading Glue is correct and HoodieHiveSyncClient should invalidate or refresh the entry in the methods that mutate the fields it serves.

Proposal

Pick one of the two readings, make both clients honor it, and record the choice in a comment on getInitialTable, because the next person who syncs one table twice in a run will hit whichever half is wrong. If (2) is chosen, the smallest change is a private invalidate(tableName) called from updateLastCommitTimeSynced, createOrReplaceTable and dropTable, the three mutators that change the cached fields, which keeps HUDI-9365's one-fetch-per-table behavior on the common path. Either way a client-level test that reads, writes and reads again would pin the chosen semantics.

Happy to send the patch once the direction is settled.

Appendix: how this surfaced

Reviewing PR #19427. Its regression test syncs once against an empty metastore, which is the one case where the redundant real-time sync is skipped anyway by the isAlreadySynced guard, so the test passes with the production change reverted. Adding a second commit and another sync round makes it fail with expected: <org.apache.hudi.hadoop.HoodieParquetInputFormat> but was: <org.apache.hudi.hadoop.realtime.HoodieParquetRealtimeInputFormat>. Working out why one round behaves differently from two is what surfaced the cache.