yihua commented on code in PR #706:
URL: https://github.com/apache/hudi-rs/pull/706#discussion_r3920589672
##########
crates/core/src/metadata/table/mod.rs:
##########
@@ -294,9 +416,181 @@ impl Table {
// where per-block fixed cost cannot amortise; on larger blocks it
overtakes
// the reader it replaces. So the ratio above is not the production
ratio, and
// nothing measured here establishes what that is.
- v2_reader::MetadataTableV2Reader::new(configs, storage)
- .read_files_partition(&file_slice, keys)
- .await
+ Ok(Some((
+ v2_reader::MetadataTableV2Reader::new(configs, storage),
+ file_slices,
+ )))
+ // NOTE: the valid-instant set is attached by
`partition_reader_with_valid_instants`,
+ // which has the data table in hand; this constructor does not.
+ }
+}
+
+/// Hudi's sentinel prefix for a metadata delta commit written outside the data
+/// timeline (`HoodieTableMetadata.SOLO_COMMIT_TIMESTAMP`).
+const SOLO_COMMIT_TIMESTAMP: &str = "00000000000000";
+
+impl Table {
+ /// The instants whose metadata log blocks may be read.
+ ///
+ /// Mirrors Java's `HoodieTableMetadataUtil.getValidInstantTimestamps`
(:2081).
+ /// This is a **set, not a window**: it has holes -- a pending data
instant is
+ /// excluded while instants either side of it are included -- and members
from
+ /// outside the data timeline entirely. See
+ ///
[`InstantRange::exact_match`](crate::timeline::selector::InstantRange::exact_match)
+ /// for why a bounded range cannot stand in for it.
+ ///
+ /// `self` is the data table; `mdt` its metadata table. Both timelines are
+ /// needed, which is why this lives here rather than on either one alone.
+ pub(crate) async fn valid_instant_timestamps(&self, mdt: &Table) ->
Result<HashSet<String>> {
+ let mut valid: HashSet<String> =
self.valid_from_completed_data_instants();
+ valid.extend(self.valid_from_mdt_delta_commits(mdt));
+ valid.extend(Self::valid_from_sentinel_commits(mdt));
+
+ // 3. Commits rolled back by the data table's rollbacks and restores.
+ // Their log blocks were written, rolled back, and re-applied, so
+ // excluding them drops records that are genuinely present.
+ //
+ // Only rollbacks newer than the earliest valid instant can have
+ // rolled back anything we hold a log block for; Java bounds the
scan
+ // the same way, falling back to the sentinel when the set is empty.
+ let earliest = valid
+ .iter()
+ .min()
+ .cloned()
+ .unwrap_or_else(|| SOLO_COMMIT_TIMESTAMP.to_string());
+ for instant in self.rollback_and_restore_instants().await? {
Review Comment:
Re-checked this against current upstream, and the premise holds:
rollbackFilterThreshold is in apache/hudi master, added by apache/hudi#18544
(89f49b18d0af, 2026-05-19) in getValidInstantTimestamps, for exactly this
latency concern. The line numbers quoted above match a pre-18544 checkout,
which is where the "not in OSS" conclusion came from; the in-code comment here
claiming Java bounds the scan the same way is likewise stale against upstream.
One sharper version of the concern: sentinel commits (00000...) enter the
valid set before the min(), so on a table whose MDT has a sentinel deltacommit
the earliest-instant bound degenerates to about zero and every completed
rollback/restore is read on each metadata read. The bounded fan-out of 8 caps
concurrency, not the count.
Still non-blocking for this PR; tracking the threshold port and the sentinel
fix as a follow-up.
--
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]
