iemejia commented on code in PR #12400:
URL: https://github.com/apache/gluten/pull/12400#discussion_r3549627434
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -176,9 +176,9 @@ object VeloxConfig extends ConfigRegistry {
val COLUMNAR_VELOX_SSD_CACHE_IO_THREADS =
buildStaticConf("spark.gluten.sql.columnar.backend.velox.ssdCacheIOThreads")
- .doc("The IO threads for cache promoting")
+ .doc("The number of IO threads for SSD cache read/write operations")
.intConf
- .createWithDefault(1)
+ .createWithDefault(4)
Review Comment:
Already fixed in the previous commit — added `.checkValue(_ > 0, "must be a
positive number")`.
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -530,10 +531,35 @@ object VeloxConfig extends ConfigRegistry {
val COLUMNAR_VELOX_FILE_HANDLE_CACHE_ENABLED =
buildStaticConf("spark.gluten.sql.columnar.backend.velox.fileHandleCacheEnabled")
.doc(
- "Disables caching if false. File handle cache should be disabled " +
- "if files are mutable, i.e. file content may change while file path
stays the same.")
+ "Enables caching of file handles to avoid repeated open/close overhead
on remote " +
+ "filesystems. Should be disabled if files are mutable, i.e. file
content may " +
+ "change while file path stays the same.")
.booleanConf
- .createWithDefault(false)
+ .createWithDefault(true)
+
+ val COLUMNAR_VELOX_NUM_CACHE_FILE_HANDLES =
+
buildStaticConf("spark.gluten.sql.columnar.backend.velox.numCacheFileHandles")
+ .doc(
+ "Maximum number of entries in the file handle cache. Each entry holds
an open " +
+ "file descriptor (local FS) or connection state (remote FS). Note
that on " +
+ "local filesystems, high values may approach the OS file descriptor
limit " +
+ "(ulimit -n). On remote object stores (S3, ABFS, GCS) entries
represent " +
+ "network connections/sockets rather than per-file OS file
descriptors, but " +
+ "they can still count toward OS resource limits (ulimit -n).")
+ .intConf
+ .checkValue(_ > 0, "must be a positive number")
+ .createWithDefault(10000)
+
+ val COLUMNAR_VELOX_FILE_HANDLE_EXPIRATION_DURATION_MS =
+
buildStaticConf("spark.gluten.sql.columnar.backend.velox.fileHandleExpirationDurationMs")
+ .doc(
+ "Expiration time in milliseconds for cached file handles. Handles not
accessed " +
+ "within this duration are evicted from the cache. This prevents
stale handles " +
+ "from accumulating (e.g., expired HDFS leases, closed remote
connections). " +
+ "A value of 0 disables TTL-based eviction.")
+ .timeConf(TimeUnit.MILLISECONDS)
+ .checkValue(_ >= 0, "must be a non-negative number (0 disables TTL-based
eviction)")
+ .createWithDefault(600000L) // 10 minutes
Review Comment:
Good catch. Reverted to `longConf`. The native side reads this via
`conf->get<int64_t>()` which can only parse plain integers, so `timeConf`
(which accepts unit-suffixed values like "10min") would silently introduce a
native init failure. This overrides the earlier `timeConf` suggestion.
##########
docs/velox-configuration.md:
##########
@@ -30,7 +30,8 @@ nav_order: 16
| spark.gluten.sql.columnar.backend.velox.directorySizeGuess
| ⚓ Static | 32KB | Deprecated, rename to
spark.gluten.sql.columnar.backend.velox.footerEstimatedSize
|
| spark.gluten.sql.columnar.backend.velox.driverSideBroadcastHashTableBuild
| 🔄 Dynamic | false | Enable driver-side broadcast hash
table build. When enabled, the hash table is built and serialized on the
driver, then broadcast to executors. When disabled, each executor builds its
own hash table from the broadcast data.
|
| spark.gluten.sql.columnar.backend.velox.enableTimestampNtzValidation
| 🔄 Dynamic | false | Enable validation fallback for
TimestampNTZ type. When true, any plan containing TimestampNTZ will fall back
to Spark execution. When false, allows native execution for TimestampNTZ scan.
|
-| spark.gluten.sql.columnar.backend.velox.fileHandleCacheEnabled
| ⚓ Static | false | Disables caching if false. File
handle cache should be disabled if files are mutable, i.e. file content may
change while file path stays the same.
|
+| spark.gluten.sql.columnar.backend.velox.fileHandleCacheEnabled
| ⚓ Static | true | Enables caching of file handles to
avoid repeated open/close overhead on remote filesystems. Should be disabled if
files are mutable, i.e. file content may change while file path stays the same.
|
+| spark.gluten.sql.columnar.backend.velox.fileHandleExpirationDurationMs
| ⚓ Static | 600000ms | Expiration time in milliseconds for
cached file handles. Handles not accessed within this duration are evicted from
the cache. This prevents stale handles from accumulating (e.g., expired HDFS
leases, closed remote connections). A value of 0 disables TTL-based eviction.
|
Review Comment:
Fixed. Reverted docs default to plain "600000" to match the `longConf` type.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
