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]

Reply via email to