yihua opened a new issue, #19429: URL: https://github.com/apache/hudi/issues/19429
### Problem The JVM-global HFile block cache (`hoodie.hfile.block.cache.enabled=true` by default since 1.1.0) keys blocks by `(filePath, offset, size)` with no content identity. That is only correct if a storage path never holds different bytes. Hudi 1.x (table version 8) does not guarantee that invariant: 1. Rollback **deletes** log files (no rollback command blocks anymore). 2. Table-service re-attempts reuse the **same instant time**. 3. On engines with deterministic write tokens (Java client is `0-0-0`; Spark falls back to it when `TaskContext` is null), the re-attempt's MDT log file gets the same `(fileId, instantTime, logVersion, writeToken)` name, so the same path is rewritten with different content. A reader that cached blocks from the first incarnation then serves them for the second. The cache TTL is expire-after-access, so a repeatedly-read stale block never expires. Observable result: `getAllFilesInPartition` returns the rolled-back attempt's files and misses the re-attempt's files. ### Why this is currently latent, not live MDT readers filter log blocks to instants completed on the data timeline (`getValidInstantTimestamps`). In the normal crash-and-retry flow the failed attempt's instant never completes, so its blocks are never read and never cached. The cache can only be poisoned when a **completed** instant is read, then unwound and re-attempted at the same instant time, e.g. manually removing a completed table-service instant to force a redo. So today the reader guards mask the violated invariant instead of the invariant being enforced. ### The design question: should Hudi restore file immutability? The narrow fix is to add content identity to the cache key (`path + length + modificationTime` from `StoragePathInfo`); a patch exists and a regression test proves it. But that patches one consumer of the broken invariant. The broader question for discussion: **should "a storage path, once written, is never rewritten with different content" be a contract Hudi guarantees by construction?** Restoring it means a re-attempt can never reuse a prior attempt's full file name, e.g. by making the write token unique per attempt on engines where it is currently deterministic. That would protect every component that assumes path identity (block caches, file-system view metadata caches, external caches such as S3 client-side caching or CDN-fronted storage), not just this one, and would eliminate the residual collision windows of the cache-key fix (same-length rewrite within filesystem mtime granularity; `StoragePathInfo` has no etag today). Costs and open points of the immutability route: - The write token format `partition-stage-attempt` is parsed numerically, so per-attempt uniqueness must stay in-format. - Rollback-before-reattempt ordering becomes a hard invariant so two same-version, different-token files never coexist as valid. - Covers MDT log appends, MDT compaction base files, and data-table compaction re-attempts. ### Proposal 1. Ship the content-aware cache key now (defense in depth; also covers path reuse Hudi's writers do not control, such as DR copy-back). 2. Discuss adopting path immutability as a 1.x storage-format contract, and if adopted, implement per-attempt write-token uniqueness. 3. Consider an etag/generation field on `StoragePathInfo` for true content identity on object stores. ### Appendix: how this was found Flaky `TestJavaHoodieBackedMetadata#testReattemptOfFailedClusteringCommit`: the test simulates "clustering succeeded in MDT, failed before data-table commit" by completing the clustering, reading the MDT (which populates the cache while the instant is valid), and then deleting the completed replacecommit. The re-attempt rewrites the MDT log file at the same path and the next lookup serves the stale cached block. The Spark variant of the test passes only because Spark's task-context-derived write token differs across attempts, which changes the file name; the cache flaw is engine-independent. A regression test at the reader-factory level (`testBlockCacheNotReusedAfterFileRewrittenAtSamePath`) reproduces the stale read without any timeline involvement. -- 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]
