advancedxy commented on code in PR #9563:
URL: https://github.com/apache/iceberg/pull/9563#discussion_r1475438902


##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/SparkReadConf.java:
##########
@@ -331,4 +331,24 @@ private long driverMaxResultSize() {
     SparkConf sparkConf = spark.sparkContext().conf();
     return sparkConf.getSizeAsBytes(DRIVER_MAX_RESULT_SIZE, 
DRIVER_MAX_RESULT_SIZE_DEFAULT);
   }
+
+  public boolean executorCacheLocalityEnabled() {
+    return executorCacheEnabled() && executorCacheLocalityEnabledInternal();
+  }
+
+  private boolean executorCacheEnabled() {
+    return confParser
+        .booleanConf()
+        .sessionConf(SparkSQLProperties.EXECUTOR_CACHE_ENABLED)
+        .defaultValue(SparkSQLProperties.EXECUTOR_CACHE_ENABLED_DEFAULT)
+        .parse();
+  }
+
+  private boolean executorCacheLocalityEnabledInternal() {

Review Comment:
   > My original approach was to enable the executor cache locality by default 
only if dynamic allocation is disabled.
   
   That's my first thought too. Then I realize what if users want to enable 
this anyway. It should be up to the users to decide. 
   
   What about log a warning when both dynamic allocation and executor cache 
locality are enabled.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to