Copilot commented on code in PR #12675:
URL: https://github.com/apache/gluten/pull/12675#discussion_r3700606321
##########
README.md:
##########
@@ -1,6 +1,5 @@
-<img src="docs/image/gluten-logo.svg" alt="Gluten" width="200">
+<img src="docs/image/gluten-logo.svg" alt="Gluten" width="260">
-# Apache Gluten
**A Middle Layer for Offloading JVM-based SQL Engines' Execution to Native
Engines**
Review Comment:
The README no longer contains a top-level "# Apache Gluten" heading (it now
starts with just the logo + tagline). This makes the repository landing page
less discoverable and can break existing anchor links/bookmarks that relied on
the H1. Consider restoring the H1 under the logo.
##########
docs/velox-backend-limitations.md:
##########
@@ -144,17 +144,10 @@ Gluten's.
Only reading with INT96 and dictionary encoding is supported. When reading
INT64 represented millisecond/microsecond timestamps, or INT96 represented
timestamps of other encodings, exceptions can occur.
-- Complex types
- - Parquet scan of nested array with struct or array as element type is not
supported in Velox (fallback behavior).
- - Parquet scan of nested map with struct as key type, or array type as value
type is not supported in Velox (fallback behavior).
-
-### CSV Read
-The header option should be true. And now we only support DatasourceV1, i.e.,
user should set `spark.sql.sources.useV1SourceList=csv`. User defined read
option is not supported, which will make CSV read fall back to vanilla Spark in
most case.
-CSV read will also fall back to vanilla Spark and log warning when user
specifies schema is different with file schema.
-
### Utilizing Map Type as Hash Keys in ColumnarShuffleExchange
Review Comment:
The CSV read limitations section was removed, but the Velox backend still
appears to only recognize CSV via DataSource V1: `getSubstraitReadFileFormatV1`
maps `CSVFileFormat` to `TextReadFormat`, while `getSubstraitReadFileFormatV2`
has no CSV case
(backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxBackend.scala:278-296).
Without a note here, readers may assume CSV (including V2) is supported.
Consider re-adding a short CSV note before this section.
##########
docs/get-started/VeloxLocalCache.md:
##########
@@ -5,16 +5,23 @@ nav_order: 7
parent: Getting-Started
---
-Velox supports a local cache when reading data from HDFS/S3/ABFS. With this
feature, Velox can asynchronously cache the data on local disk when reading
from remote storage and future read requests on previously cached blocks will
be serviced from local cache files. To enable the local caching feature, the
following configurations are required:
+Velox supports local caching when reading data from HDFS/S3/ABFS. With this
feature, Velox asynchronously caches data on
+local disk when reading from remote storage, and future read requests for
previously cached blocks are served from the
+local cache files. To enable local caching, the following configurations are
required:
```
-spark.gluten.sql.columnar.backend.velox.cacheEnabled // enable or disable
velox cache, default false.
-spark.gluten.sql.columnar.backend.velox.memCacheSize // the total size of
in-mem cache, default is 128MB.
-spark.gluten.sql.columnar.backend.velox.ssdCachePath // the folder to
store the cache files, default is "/tmp".
-spark.gluten.sql.columnar.backend.velox.ssdCacheSize // the total size of
the SSD cache, default is 128MB. Velox will do in-mem cache only if this value
is 0.
-spark.gluten.sql.columnar.backend.velox.ssdCacheShards // the shards of the
SSD cache, default is 1.
-spark.gluten.sql.columnar.backend.velox.ssdCacheIOThreads // the number of IO
threads for SSD cache read/write operations, default is 4. Velox will try to do
"read-ahead" if this value is bigger than 1
-spark.gluten.sql.columnar.backend.velox.ssdODirect // enable or disable
O_DIRECT on cache write, default false.
+spark.gluten.sql.columnar.backend.velox.cacheEnabled // Enable Velox
cache. Default: false.
+spark.gluten.sql.columnar.backend.velox.memCacheSize // In-memory cache
size. Default: 128MB.
+spark.gluten.sql.columnar.backend.velox.ssdCachePath // Folder to store
cache files, preferably on SSD. Default: "/tmp".
+spark.gluten.sql.columnar.backend.velox.ssdCacheSize // SSD cache size.
Memory-only caching is used when set to 0. Default: 128MB.
+spark.gluten.sql.columnar.backend.velox.ssdCacheShards // Number of SSD
cache shards. Default: 1.
+spark.gluten.sql.columnar.backend.velox.ssdCacheIOThreads // Number of IO
threads for SSD cache read/write. Enables read-ahead when > 1. Default: 4.
+spark.gluten.sql.columnar.backend.velox.loadQuantum // Load quantum
size. Must be set to 8MB when Velox cache is enabled, otherwise Velox fails.
Default: 256MB.
Review Comment:
The local-cache docs state `loadQuantum` "must be set to 8MB ... otherwise
Velox fails", but the config definition documents it as "8MB at most" when
cache is enabled (i.e., <= 8MB), not necessarily exactly 8MB (see
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala around
LOAD_QUANTUM). The wording here should match the actual constraint.
##########
dev/release/build-release.sh:
##########
@@ -62,7 +62,7 @@ do
-Piceberg,delta,hudi,paimon -DskipTests
done
-for spark_version in 4.0
+for spark_version in 4.0 4.1
do
${GLUTEN_HOME}/build/mvn clean install -Pjava-17 -Pscala-2.13
-Pbackends-velox -Pspark-${spark_version} -Piceberg,paimon -DskipTests
done
Review Comment:
This PR is titled "[DOC] ..." but also changes release build behavior by
adding Spark 4.1 to the release build matrix. Please either split this into a
separate non-doc PR, or update the PR title/scope to reflect the script change.
--
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]
