jackylee-ch commented on code in PR #12549:
URL: https://github.com/apache/gluten/pull/12549#discussion_r3949890841
##########
gluten-substrait/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##########
@@ -474,119 +473,154 @@ object GlutenConfig extends ConfigRegistry {
val SPARK_UNSAFE_SORTER_SPILL_READER_BUFFER_SIZE =
"spark.unsafe.sorter.spill.reader.buffer.size"
val SPARK_SHUFFLE_SPILL_DISK_WRITE_BUFFER_SIZE =
"spark.shuffle.spill.diskWriteBufferSize"
val SPARK_SHUFFLE_SPILL_COMPRESS = "spark.shuffle.spill.compress"
- val SPARK_SHUFFLE_SPILL_COMPRESS_DEFAULT: Boolean = true
+ // The codec `spark.gluten.sql.columnar.shuffle.codec` falls back to, and
its Spark default.
+ val SPARK_IO_COMPRESSION_CODEC = "spark.io.compression.codec"
+ val SPARK_IO_COMPRESSION_CODEC_DEFAULT = "lz4"
val SPARK_MAX_BROADCAST_TABLE_SIZE = "spark.sql.maxBroadcastTableSize"
- def get: GlutenConfig = {
+ override def get: GlutenConfig = {
new GlutenConfig(GlutenCoreConfig.activeSQLConf)
}
def prefixOf(backendName: String): String =
s"spark.gluten.sql.columnar.backend.$backendName"
def prefixSessionOf(backendName: String): String =
s"spark.gluten.$backendName"
- private lazy val nativeKeys = Set(
- DEBUG_ENABLED.key,
- BENCHMARK_SAVE_DIR.key,
- GlutenCoreConfig.COLUMNAR_TASK_OFFHEAP_SIZE_IN_BYTES.key,
- COLUMNAR_MAX_BATCH_SIZE.key,
- SHUFFLE_WRITER_BUFFER_SIZE.key,
- COLUMNAR_CUDF_ENABLED.key,
- SQLConf.LEGACY_SIZE_OF_NULL.key,
- SQLConf.LEGACY_STATISTICAL_AGGREGATE.key,
- SQLConf.JSON_GENERATOR_IGNORE_NULL_FIELDS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_EXPECTED_NUM_ITEMS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_BITS.key,
- SQLConf.RUNTIME_BLOOM_FILTER_MAX_NUM_ITEMS.key,
- "spark.io.compression.codec",
- "spark.sql.decimalOperations.allowPrecisionLoss",
- // s3 config
- SPARK_S3_ACCESS_KEY,
- SPARK_S3_SECRET_KEY,
- SPARK_S3_ENDPOINT,
- SPARK_S3_CONNECTION_SSL_ENABLED,
- SPARK_S3_PATH_STYLE_ACCESS,
- SPARK_S3_USE_INSTANCE_CREDENTIALS,
- SPARK_S3_IAM,
- SPARK_S3_IAM_SESSION_NAME,
- SPARK_S3_RETRY_MAX_ATTEMPTS,
- SPARK_S3_CONNECTION_MAXIMUM,
- SPARK_S3_ENDPOINT_REGION,
- SPARK_S3_AWS_IMDS_ENABLED,
- "spark.gluten.velox.fs.s3a.retry.mode",
- "spark.gluten.velox.awsSdkLogLevel",
- "spark.gluten.velox.s3UseProxyFromEnv",
- "spark.gluten.velox.s3PayloadSigningPolicy",
- "spark.gluten.velox.s3LogLocation",
Review Comment:
Double-checked the `VeloxS3.md` table you linked. It has five problems, all
of which predate this PR — none is introduced by it, so I would rather fix them
in a doc-only PR than widen this diff. Listing them here so they are on record:
The "Gluten new parameters" table states **native's own fallbacks**, not the
values Gluten actually delivers:
| Row | Doc says | Gluten actually delivers | Where |
|---|---|---|---|
| `path.style.access` | `false` | `true` | `GlutenConfig.scala:625`
`createWithDefault(true)`; native falls back to `false` at
`ConfigExtractor.cc:74` |
| `connection.maximum` | `25` | `15` | `createWithDefault(15)`; native falls
back to `25` at `ConfigExtractor.cc:77` |
| `retry.limit` | `(none)` | `20` | `createWithDefault(20)`; native has no
fallback |
Plus two more:
- The table gives bare suffixes with no statement that they take the
`spark.hadoop.fs.s3a.` prefix, which is easy to misread as literal conf names.
- `instance.credentials` in the doc matches what native looks up
(`ConfigExtractor.cc:80`), but Gluten's own constant is
`fs.s3a.use.instance.credentials` (`GlutenConfig.scala:437`). One of the two
spellings has never worked, and it is the Scala one — so setting
`spark.hadoop.fs.s3a.use.instance.credentials` has never reached native's
reader. Also pre-existing, and fixing it changes behavior for anyone who
happened to set the working spelling, so it needs its own PR and a note.
On your other point — yes, the config itself stays; only the separate list
of native keys goes away. As the PR description now spells out,
`spark.gluten.velox.s3UseProxyFromEnv` and `s3PayloadSigningPolicy` actually
gain from this: base had them only as bare string literals inside `nativeKeys`,
and they are real, documented, type-checked conf entries now.
--
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]
