voonhous commented on code in PR #19849:
URL: https://github.com/apache/hudi/pull/19849#discussion_r3951381224
##########
hudi-common/src/main/java/org/apache/hudi/core/read/BaseHoodieTableFileIndex.java:
##########
@@ -97,7 +97,6 @@ public abstract class BaseHoodieTableFileIndex implements
AutoCloseable {
@Getter(AccessLevel.PROTECTED)
private final String[] partitionColumns;
- @Getter
protected final HoodieMetadataConfig metadataConfig;
Review Comment:
Addressed in `fde0f7d8`. `TestLocalIndex` now takes the RO-path-filter flag,
and an 8-row truth table asserts `metadataConfig.isEnabled()` through the real
constructor rather than a planted field.
Since the complaint was that the removed test discriminated nothing, I
mutation-checked this one instead of trusting a green run: dropping each of the
three conjuncts from the production expression in turn makes exactly one row
fail (the RO-filter conjunct, `isFilesPartitionAvailable`, and the
`hoodie.metadata.enable` term).
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieBaseRelation.scala:
##########
@@ -244,28 +233,16 @@ abstract class HoodieBaseRelation(val sqlContext:
SQLContext,
/**
* NOTE: PLEASE READ THIS CAREFULLY
*
- * Even though [[HoodieFileIndex]] initializes eagerly listing all of the
files w/in the given Hudi table,
- * this variable itself is _lazy_ (and have to stay that way) which
guarantees that it's not initialized, until
+ * Even though [[HoodieFileIndex]] does eager work on construction (it opens
the metadata-table reader
+ * and reloads the active timeline, and lists every file in the table when
+ * `hoodie.datasource.read.file.index.listing.mode` is `eager`; it defaults
to `lazy`), this variable
+ * itself is _lazy_ (and have to stay that way) which guarantees that it's
not initialized, until
* it's actually accessed
*/
lazy val fileIndex: HoodieFileIndex =
HoodieFileIndex(sparkSession, metaClient, Some(tableStructSchema),
optParams,
FileStatusCache.getOrCreate(sparkSession), shouldIncludeLogFiles())
Review Comment:
Documented in the Impact section: on the MOR incremental non-full-scan path
the `metaClient` serialized into the RDD is no longer preceded by the file
index's `reloadActiveTimeline()`. The instant range each task reads is fixed
before `composeRDD`, so it is a timing change rather than a visibility one.
--
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]