yihua commented on code in PR #13927:
URL: https://github.com/apache/hudi/pull/13927#discussion_r2386903216


##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -858,41 +853,4 @@ private HoodiePairData<String, String> 
readSecondaryIndexDataTableRecordKeysWith
     return readIndexRecords(rawKeys, partitionName, Option.empty())
         .mapToPair(hoodieRecord -> 
SecondaryIndexKeyUtils.getSecondaryKeyRecordKeyPair(hoodieRecord.getRecordKey()));
   }
-
-  /**
-   * Derive necessary properties for FG reader.
-   */
-  TypedProperties buildFileGroupReaderProperties(HoodieMetadataConfig 
metadataConfig, boolean shouldReuse) {
-    HoodieCommonConfig commonConfig = HoodieCommonConfig.newBuilder()
-        .fromProperties(metadataConfig.getProps()).build();
-    TypedProperties props = new TypedProperties();
-    props.setProperty(
-        MAX_MEMORY_FOR_MERGE.key(),
-        Long.toString(metadataConfig.getMaxReaderMemory()));
-    props.setProperty(
-        SPILLABLE_MAP_BASE_PATH.key(),
-        metadataConfig.getSplliableMapDir());
-    props.setProperty(
-        SPILLABLE_DISK_MAP_TYPE.key(),
-        commonConfig.getSpillableDiskMapType().name());
-    props.setProperty(
-        DISK_MAP_BITCASK_COMPRESSION_ENABLED.key(),
-        Boolean.toString(commonConfig.isBitCaskDiskMapCompressionEnabled()));
-    if (shouldReuse) {
-      setHFileBlockCacheProps(props);
-    } else {
-      props.setProperty(HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED.key(),
-          
metadataConfig.getStringOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED));
-    }
-    props.setProperty(HoodieReaderConfig.HFILE_BLOCK_CACHE_SIZE.key(),
-        
metadataConfig.getStringOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_SIZE));
-    props.setProperty(HoodieReaderConfig.HFILE_BLOCK_CACHE_TTL_MINUTES.key(),
-        
metadataConfig.getStringOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_TTL_MINUTES));
-    return props;

Review Comment:
   If `shouldReuse` is set, the cache should be enabled regardless of the 
config.



##########
hudi-common/src/main/java/org/apache/hudi/common/util/ConfigUtils.java:
##########
@@ -748,8 +754,28 @@ public static void deleteProperties(Properties current, 
Properties deleted) {
   public static HoodieConfig getReaderConfigs(StorageConfiguration<?> 
storageConf) {
     HoodieConfig config = new HoodieConfig();
     config.setAll(DEFAULT_HUDI_CONFIG_FOR_READER.getProps());
-    config.setValue(USE_NATIVE_HFILE_READER,
-        Boolean.toString(storageConf.getBoolean(USE_NATIVE_HFILE_READER.key(), 
USE_NATIVE_HFILE_READER.defaultValue())));
+    return config;
+  }
+
+  /**
+   * Apply HFile cache configurations from options to a HoodieConfig.
+   * This method extracts HFile cache-related settings from the provided 
options map
+   * and applies them to the given HoodieConfig instance.
+   *
+   * @param options Map of options containing HFile cache configurations
+   * @return HoodieConfig with HFile reader configurations
+   */
+  public static HoodieReaderConfig getHFileCacheConfigs(Map<String, String> 
options) {
+    HoodieReaderConfig config = new HoodieReaderConfig();
+    config.setValue(HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED,
+        
options.getOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED.key(),

Review Comment:
   nit: use `getStringWithAltKeys(options, 
HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED)`, etc.



##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -858,41 +853,4 @@ private HoodiePairData<String, String> 
readSecondaryIndexDataTableRecordKeysWith
     return readIndexRecords(rawKeys, partitionName, Option.empty())
         .mapToPair(hoodieRecord -> 
SecondaryIndexKeyUtils.getSecondaryKeyRecordKeyPair(hoodieRecord.getRecordKey()));
   }
-
-  /**
-   * Derive necessary properties for FG reader.
-   */
-  TypedProperties buildFileGroupReaderProperties(HoodieMetadataConfig 
metadataConfig, boolean shouldReuse) {
-    HoodieCommonConfig commonConfig = HoodieCommonConfig.newBuilder()
-        .fromProperties(metadataConfig.getProps()).build();
-    TypedProperties props = new TypedProperties();
-    props.setProperty(
-        MAX_MEMORY_FOR_MERGE.key(),
-        Long.toString(metadataConfig.getMaxReaderMemory()));
-    props.setProperty(
-        SPILLABLE_MAP_BASE_PATH.key(),
-        metadataConfig.getSplliableMapDir());
-    props.setProperty(
-        SPILLABLE_DISK_MAP_TYPE.key(),
-        commonConfig.getSpillableDiskMapType().name());
-    props.setProperty(
-        DISK_MAP_BITCASK_COMPRESSION_ENABLED.key(),
-        Boolean.toString(commonConfig.isBitCaskDiskMapCompressionEnabled()));
-    if (shouldReuse) {
-      setHFileBlockCacheProps(props);
-    } else {
-      props.setProperty(HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED.key(),
-          
metadataConfig.getStringOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_ENABLED));
-    }
-    props.setProperty(HoodieReaderConfig.HFILE_BLOCK_CACHE_SIZE.key(),
-        
metadataConfig.getStringOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_SIZE));
-    props.setProperty(HoodieReaderConfig.HFILE_BLOCK_CACHE_TTL_MINUTES.key(),
-        
metadataConfig.getStringOrDefault(HoodieReaderConfig.HFILE_BLOCK_CACHE_TTL_MINUTES));
-    return props;

Review Comment:
   Fixed.



-- 
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