You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "gerashegalov (via GitHub)" <gi...@apache.org> on 2026/04/29 08:51:45 UTC
[PR] Spark: Add session-level split size override [iceberg]
gerashegalov opened a new pull request, #16154:
URL: https://github.com/apache/iceberg/pull/16154
Closes #16153
### What changes were made in this PR?
Add a new Spark session configuration key `spark.sql.iceberg.split-size` that allows overriding
the `read.split.target-size` table property at the session level without requiring DDL changes
to table metadata or source code changes to read call sites.
This is particularly useful when GPU and CPU workloads read the same Iceberg table
concurrently: GPU sessions benefit from significantly larger splits (e.g. 2GB) while CPU
sessions perform better with the default 128MB. Hardware accelerators like
[RAPIDS Accelerator for Apache Spark](https://nvidia.github.io/spark-rapids/) are designed as
drop-in replacements requiring no application code changes, so a session-level knob is essential.
### Changes
**All Spark shims (v3.4, v3.5, v4.0):**
- `SparkSQLProperties`: add `SPLIT_SIZE = "spark.sql.iceberg.split-size"` constant
- `SparkReadConf`: add `.sessionConf(SparkSQLProperties.SPLIT_SIZE)` to both `splitSize()` and
`splitSizeOption()` parser chains; update Javadoc to document 5-level precedence
- `SparkConfParser`: store `Table.name()` as `tableName` and in `ConfParser.parse()` try a
table-qualified session key (`<key>.<tableName>`) before the global session key
**v3.5 only:**
- `TestSparkWriteConf`: add 4 tests for table-scoped session conf resolution
### Resolution precedence
1. Read option (`split-size`)
2. Table-scoped session conf (`spark.sql.iceberg.split-size.<catalog>.<db>.<table>`)
3. Global session conf (`spark.sql.iceberg.split-size`)
4. Table property (`read.split.target-size`)
5. Default (128MB)
### How was this patch tested?
4 new unit tests in `TestSparkWriteConf` (v3.5):
- table-scoped session key takes precedence over global
- global session key works when no table-scoped key is set
- read option takes precedence over table-scoped session key
- table-scoped session key takes precedence over table property
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "liucao-dd (via GitHub)" <gi...@apache.org>.
liucao-dd commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3398915313
##########
docs/docs/spark-configuration.md:
##########
@@ -199,6 +199,8 @@ val spark = SparkSession.builder()
| spark.sql.iceberg.data-planning-mode | AUTO | Scan planning mode for data files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.delete-planning-mode | AUTO | Scan planning mode for delete files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.advisory-partition-size | Table default | Advisory size (bytes) used for writing to the Table when Spark's Adaptive Query Execution is enabled. Used to size output files |
+| spark.sql.iceberg.split-size | Table default | Overrides `read.split.target-size` for scan planning. Session values are honored like read options and disable adaptive split-size adjustment |
+| spark.sql.iceberg.split-size.<table-name> | Global session default | Table-scoped split size override using the fully qualified table name as a suffix |
Review Comment:
Could we consider making table-scoped session configs a generic identity-first pattern instead of a split-size-specific suffix? Having the table name at the end of the config seems confusing.
An alternative is to resolve table-scoped session configs as:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.<setting-suffix>`
For this config, if the global key becomes `spark.sql.iceberg.read.split-size`, the table-scoped key would be:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.read.split-size`
That keeps the table identity together, then applies the same setting suffix used by the global session config, making it a generic pattern to support table specific session config.
It would be a slightly larger change admittedly, and i'm happy to help or open a separate PR just to add the table level generic session config support if needed.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4972605294
Merged, thanks @gerashegalov and all for review !
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho merged PR #16154:
URL: https://github.com/apache/iceberg/pull/16154
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4781047561
@anuragmantri Could you check this out, you are more deeply involved in the Spark code these days than me
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "liucao-dd (via GitHub)" <gi...@apache.org>.
liucao-dd commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4684535424
Hi folks, author of https://github.com/apache/iceberg/pull/16185 here - leaving that one as closed and +1 on the general feature added on this PR. Given SQL statements such as MERGE INTO, UPDATE, and DELETE do not expose DataFrame read options, and changing table properties affects all readers, having a session-level override would help standard use cases as well, not only GPU users.
I do have other thoughts on naming / interface implementation detail, will leave separate comments
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3461043977
##########
docs/docs/spark-configuration.md:
##########
@@ -199,6 +199,8 @@ val spark = SparkSession.builder()
| spark.sql.iceberg.data-planning-mode | AUTO | Scan planning mode for data files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.delete-planning-mode | AUTO | Scan planning mode for delete files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.advisory-partition-size | Table default | Advisory size (bytes) used for writing to the Table when Spark's Adaptive Query Execution is enabled. Used to size output files |
+| spark.sql.iceberg.split-size | Table default | Overrides `read.split.target-size` for scan planning. Session values are honored like read options and disable adaptive split-size adjustment |
+| spark.sql.iceberg.split-size.<table-name> | Global session default | Table-scoped split size override using the fully qualified table name as a suffix |
Review Comment:
I'm not super excited about allowing for table level defaults through the session parameters. Feels like we are working around an issue (the inability to set config per identifier). Are we ok with just allowing the global defautl there?
The alternative proposed here means we are essentially adding a completely new paradigm to parsing these config entries.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3574847337
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkReadConf.java:
##########
@@ -39,13 +39,16 @@
*
* <ol>
* <li>Read options
- * <li>Session configuration
+ * <li>Session configuration (e.g. {@code spark.sql.iceberg.read.split-size})
* <li>Table metadata
* </ol>
*
* The most specific value is set in read options and takes precedence over all other configs. If no
- * read option is provided, this class checks the session configuration for any overrides. If no
- * applicable value is found in the session configuration, this class uses the table metadata.
+ * read option is provided, this class checks the session configuration. If no applicable value is
+ * found in the session configuration, this class uses the table metadata.
+ *
+ * <p>For split size, session overrides are treated like explicit read options and disable adaptive
Review Comment:
seems weird on the class, can you put this on the javadoc of the method?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3461043977
##########
docs/docs/spark-configuration.md:
##########
@@ -199,6 +199,8 @@ val spark = SparkSession.builder()
| spark.sql.iceberg.data-planning-mode | AUTO | Scan planning mode for data files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.delete-planning-mode | AUTO | Scan planning mode for delete files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.advisory-partition-size | Table default | Advisory size (bytes) used for writing to the Table when Spark's Adaptive Query Execution is enabled. Used to size output files |
+| spark.sql.iceberg.split-size | Table default | Overrides `read.split.target-size` for scan planning. Session values are honored like read options and disable adaptive split-size adjustment |
+| spark.sql.iceberg.split-size.<table-name> | Global session default | Table-scoped split size override using the fully qualified table name as a suffix |
Review Comment:
I'm not super excited about allowing for table level defaults through the session parameters. Feels like we are working around an issue (the inability to set config per identifier). Are we ok with just allowing the global default here?
The alternative proposed here means we are essentially adding a completely new paradigm to parsing these config entries.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "liucao-dd (via GitHub)" <gi...@apache.org>.
liucao-dd commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3398889820
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java:
##########
@@ -70,6 +70,9 @@ private SparkSQLProperties() {}
// Overrides the advisory partition size
public static final String ADVISORY_PARTITION_SIZE = "spark.sql.iceberg.advisory-partition-size";
+ // Overrides the split target size for scan planning
+ public static final String SPLIT_SIZE = "spark.sql.iceberg.split-size";
Review Comment:
Should this be `spark.sql.iceberg.read.split-size` instead?
The closest merged precedent is #16088, which added the split-planning session configs under the read namespace:
- `spark.sql.iceberg.read.adaptive-split-size.enabled`
- `spark.sql.iceberg.read.adaptive-split-size.parallelism`
Since this is the fixed split-size counterpart to adaptive split sizing, keeping it under `spark.sql.iceberg.read.*` seems more consistent with the naming convention that has already landed. It also leaves room for any future non-read `split-size` concept without overloading the top-level `spark.sql.iceberg.split-size` name.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "liucao-dd (via GitHub)" <gi...@apache.org>.
liucao-dd commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3398915313
##########
docs/docs/spark-configuration.md:
##########
@@ -199,6 +199,8 @@ val spark = SparkSession.builder()
| spark.sql.iceberg.data-planning-mode | AUTO | Scan planning mode for data files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.delete-planning-mode | AUTO | Scan planning mode for delete files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.advisory-partition-size | Table default | Advisory size (bytes) used for writing to the Table when Spark's Adaptive Query Execution is enabled. Used to size output files |
+| spark.sql.iceberg.split-size | Table default | Overrides `read.split.target-size` for scan planning. Session values are honored like read options and disable adaptive split-size adjustment |
+| spark.sql.iceberg.split-size.<table-name> | Global session default | Table-scoped split size override using the fully qualified table name as a suffix |
Review Comment:
Could we consider making table-scoped session configs a generic identity-first pattern instead of a split-size-specific suffix?
The current shape, `spark.sql.iceberg.split-size.<table-name>`, puts the setting before the table identity. That works for this one config, but it is harder to generalize and can be confusing once more session-backed settings become table-scoped.
An alternative is to resolve table-scoped session configs as:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.<setting-suffix>`
For this config, if the global key becomes `spark.sql.iceberg.read.split-size`, the table-scoped key would be:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.read.split-size`
That keeps the table identity together, then applies the same setting suffix used by the global session config. It should also avoid reverse-parsing ambiguity if the key is constructed from the resolved catalog + Spark `Identifier` and looked up exactly.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "liucao-dd (via GitHub)" <gi...@apache.org>.
liucao-dd commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3398889820
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java:
##########
@@ -70,6 +70,9 @@ private SparkSQLProperties() {}
// Overrides the advisory partition size
public static final String ADVISORY_PARTITION_SIZE = "spark.sql.iceberg.advisory-partition-size";
+ // Overrides the split target size for scan planning
+ public static final String SPLIT_SIZE = "spark.sql.iceberg.split-size";
Review Comment:
Should this be `spark.sql.iceberg.read.split-size` instead?
The closest merged precedent is #16088, which added the split-planning session configs under the read namespace:
- `spark.sql.iceberg.read.adaptive-split-size.enabled`
- `spark.sql.iceberg.read.adaptive-split-size.parallelism`
Since this is the fixed split-size counterpart to adaptive split sizing, keeping it under `spark.sql.iceberg.read.*` seems more consistent
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4628042882
Context for reviewers - this capability has prior history worth reconciling before a deep review.
The same feature was proposed twice before and closed without merging (#10336 and its revival #13677). On #10336, @szehon-ho objected to a bare global `spark.sql.iceberg.split-size` key on design grounds: "The problem with this is that it does lead to some ambiguity as to what table the config is applying to (many queries read from several tables, for example)" (https://github.com/apache/iceberg/pull/10336#issuecomment-2182038718). He pointed at per-scan SQL OPTIONS on the Spark side as the alternative (https://github.com/apache/spark/pull/46707) and later noted it had become possible there (https://github.com/apache/iceberg/pull/10336#issuecomment-2219566750).
This PR's table-scoped key (`spark.sql.iceberg.split-size.<table-name>`) directly answers that multi-table ambiguity. The global tier (`spark.sql.iceberg.split-size`) it also adds reintroduces the same ambiguity for queries reading several tables, so a maintainer call on whether the global tier should stay would help before deeper review.
There is also a competing open PR #16185 proposing the same capability with different naming (`spark.sql.iceberg.split.target-size`, plus lookback / open-file-cost / adaptive knobs, mirroring the table-property names). It is currently stale. Worth deciding which naming and scope to converge on before either is reviewed in depth.
Two related notes for whoever picks this up:
- whether per-scan SQL OPTIONS (now available in Spark) already cover the original use case is worth re-confirming.
- the resolution change here is generic - it adds the `.<table-name>` table-scoped suffix to every `spark.sql.iceberg.*` session conf, not only split-size.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3360312152
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkConfParser.java:
##########
@@ -278,6 +281,20 @@ protected T parse(Function<String, T> conversion, T defaultValue) {
}
if (sessionConfName != null) {
+ if (tableName != null) {
+ String tableSessionConfName = sessionConfName + "." + tableName;
+ String tableSessionConfValue = sessionConf.get(tableSessionConfName, null);
+ if (tableSessionConfValue != null) {
+ return conversion.apply(tableSessionConfValue);
+ }
+
+ String sparkTableSessionConfValue =
+ sessionConf.get(toCamelCase(tableSessionConfName), null);
Review Comment:
`toCamelCase` is applied to the full composed key including the `table.name()` suffix, so any hyphen in the catalog/database/table name (legal via backticks) gets folded into camelCase and this fallback can never match the key a user would set. The primary kebab lookup at line 286 uses the raw name and is fine. If the camelCase variant is only meant to cover the conf-name part, camel-case just that: `sessionConf.get(toCamelCase(sessionConfName) + "." + tableName, null)`.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "gerashegalov (via GitHub)" <gi...@apache.org>.
gerashegalov commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4920163938
> Also, I'm not a GPU programmer but I'm curious, isn't having a uniform split size for all the tables involved in a session be better for GPU bandwidth utilization
My starting point was actually a single global default size. I don't see per-table tuning as a GPU-specific concern. It is like with the CPU the ideal split size depends on compression ratios, table size (e.g. dim vs fact in a join). The GPU specific part is that you often want it much bigger relative to the CPU setting.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "gerashegalov (via GitHub)" <gi...@apache.org>.
gerashegalov commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4673971797
Thanks for reviewing @wombatu-kun , especially for the context.
Having a discussion about https://github.com/apache/iceberg/issues/16153 with the community is the most important part. I am open to alternative user-friendly solutions that do not involve code change on the application side or fine tuning individual tables of which it could be hundreds or even thousands.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3360264597
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkSQLProperties.java:
##########
@@ -70,6 +70,9 @@ private SparkSQLProperties() {}
// Overrides the advisory partition size
public static final String ADVISORY_PARTITION_SIZE = "spark.sql.iceberg.advisory-partition-size";
+ // Overrides the split target size for scan planning
+ public static final String SPLIT_SIZE = "spark.sql.iceberg.split-size";
Review Comment:
This adds a user-facing session conf but nothing in `docs/docs/spark-configuration.md`, where every other `spark.sql.iceberg.*` conf is listed (the adaptive-split confs from #16088 are at lines 210-211). Add `spark.sql.iceberg.split-size`, and the table-scoped `spark.sql.iceberg.split-size.<table-name>` form, to that table.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "wombatu-kun (via GitHub)" <gi...@apache.org>.
wombatu-kun commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3360259547
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkReadConf.java:
##########
@@ -156,13 +160,18 @@ public int orcBatchSize() {
}
public Long splitSizeOption() {
- return confParser.longConf().option(SparkReadOptions.SPLIT_SIZE).parseOptional();
+ return confParser
+ .longConf()
+ .option(SparkReadOptions.SPLIT_SIZE)
+ .sessionConf(SparkSQLProperties.SPLIT_SIZE)
Review Comment:
`splitSizeOption()` is the gate term in `SparkScan.java:370` (`splitSizeOption() == null && adaptiveSplitSizeEnabled()`), so adding `SPLIT_SIZE` here makes setting the session conf disable adaptive split sizing, which defaults to on. The table-property path leaves `splitSizeOption()` null and keeps adaptive sizing enabled, so the same split-size value behaves differently depending on whether it comes from this session conf or `read.split.target-size`. If that is intended (honor an explicit session split size exactly, as the read option does), document it in the precedence javadoc and add a test for the interaction; otherwise the session conf should not suppress adaptive sizing.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "gerashegalov (via GitHub)" <gi...@apache.org>.
gerashegalov commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4919220759
Updated the branch to address the review feedback:
- Removed the table-scoped session configuration mechanism from this PR.
- Narrowed the code changes to Spark 4.1 only.
- Renamed the session key to `spark.sql.iceberg.read.split-size` to align with the existing read/adaptive split-size configs.
- Kept docs and tests explicit that a session split-size override behaves like a read option and disables adaptive split-size adjustment.
Backports or table-scoped behavior can be handled separately if needed.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4780989965
Ah I missed - https://github.com/apache/iceberg/pull/16154#issuecomment-4628042882 which has this same commentary.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "anuragmantri (via GitHub)" <gi...@apache.org>.
anuragmantri commented on PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#issuecomment-4786006519
I just took a look at the PR and all the historical context. The motivation is clear to me. I echo @RussellSpitzer and @wombatu-kun's comments.
The global session conf is the right starting point. Every existing Iceberg session conf (`vectorization.enabled`, `data-planning-mode`, `advisory-partition-size`, `aggregate-push-down.enabled`) is a global key. This follows that pattern exactly and is easy to reason about.
I'd also suggest removing the table-scoped mechanism from this PR. A few reasons:
1. Spark itself has no per-table session config pattern anywhere. The closest analog is spark.sql.catalog.<name>.* for catalog plugin, and that uses prefix-based extraction (iterate all configs, regex-match the prefix, strip it). There is no suffix approach.
2. Every `sessionConf()` caller in SparkConfParser now silently supports table-scoping. Which is a larger behavior change that needs more discussion as the earlier reviewers pointed out.
3. Table.name() format varies across catalog implementations. The user must know exactly what string their catalog returns to construct the session key, which is fragile.
I know this design was rejected earlier citing table ambiguity, but since then Spark has added SQL options in SELECT (https://github.com/apache/spark/pull/46707) and INSERT (https://github.com/apache/spark/pull/47591). I think we should add the support for MERGE, UPDATE and DELETE statements as well in Spark. I can attempt to make these changes but I believe that will be the cleanest way to do this.
Also, I'm not a GPU programmer but I'm curious, isn't having a uniform split size for all the tables involved in a session be better for GPU bandwidth utilization?
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "gerashegalov (via GitHub)" <gi...@apache.org>.
gerashegalov commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3575599729
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkReadConf.java:
##########
@@ -39,13 +39,16 @@
*
* <ol>
* <li>Read options
- * <li>Session configuration
+ * <li>Session configuration (e.g. {@code spark.sql.iceberg.read.split-size})
* <li>Table metadata
* </ol>
*
* The most specific value is set in read options and takes precedence over all other configs. If no
- * read option is provided, this class checks the session configuration for any overrides. If no
- * applicable value is found in the session configuration, this class uses the table metadata.
+ * read option is provided, this class checks the session configuration. If no applicable value is
+ * found in the session configuration, this class uses the table metadata.
+ *
+ * <p>For split size, session overrides are treated like explicit read options and disable adaptive
Review Comment:
Good point @szehon-ho. I removed the split-size-specific Javadoc entirely to keep this consistent with the other config accessors. The behavior is still covered by the docs and test.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark: Add session-level split size override [iceberg]
Posted by "liucao-dd (via GitHub)" <gi...@apache.org>.
liucao-dd commented on code in PR #16154:
URL: https://github.com/apache/iceberg/pull/16154#discussion_r3398915313
##########
docs/docs/spark-configuration.md:
##########
@@ -199,6 +199,8 @@ val spark = SparkSession.builder()
| spark.sql.iceberg.data-planning-mode | AUTO | Scan planning mode for data files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.delete-planning-mode | AUTO | Scan planning mode for delete files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.advisory-partition-size | Table default | Advisory size (bytes) used for writing to the Table when Spark's Adaptive Query Execution is enabled. Used to size output files |
+| spark.sql.iceberg.split-size | Table default | Overrides `read.split.target-size` for scan planning. Session values are honored like read options and disable adaptive split-size adjustment |
+| spark.sql.iceberg.split-size.<table-name> | Global session default | Table-scoped split size override using the fully qualified table name as a suffix |
Review Comment:
Could we consider making table-scoped session configs a generic identity-first pattern instead of a split-size-specific suffix? Having the table name at the end of the config seems confusing.
An alternative is to resolve table-scoped session configs as:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.<setting-suffix>`
For this config, if the global key becomes `spark.sql.iceberg.read.split-size`, the table-scoped key would be:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.read.split-size`
That keeps the table identity together, then applies the same setting suffix used by the global session config, making it a generic pattern to support table specific session config.
It would be a larger scope admittedly, and i'm happy to help if needed
##########
docs/docs/spark-configuration.md:
##########
@@ -199,6 +199,8 @@ val spark = SparkSession.builder()
| spark.sql.iceberg.data-planning-mode | AUTO | Scan planning mode for data files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.delete-planning-mode | AUTO | Scan planning mode for delete files (`AUTO`, `LOCAL`, `DISTRIBUTED`) |
| spark.sql.iceberg.advisory-partition-size | Table default | Advisory size (bytes) used for writing to the Table when Spark's Adaptive Query Execution is enabled. Used to size output files |
+| spark.sql.iceberg.split-size | Table default | Overrides `read.split.target-size` for scan planning. Session values are honored like read options and disable adaptive split-size adjustment |
+| spark.sql.iceberg.split-size.<table-name> | Global session default | Table-scoped split size override using the fully qualified table name as a suffix |
Review Comment:
Could we consider making table-scoped session configs a generic identity-first pattern instead of a split-size-specific suffix? Having the table name at the end of the config seems confusing.
An alternative is to resolve table-scoped session configs as:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.<setting-suffix>`
For this config, if the global key becomes `spark.sql.iceberg.read.split-size`, the table-scoped key would be:
` spark.sql.iceberg.<catalog>.<namespace...>.<table>.read.split-size`
That keeps the table identity together, then applies the same setting suffix used by the global session config, making it a generic pattern to support table specific session config.
It would be a slightly larger change admittedly, and i'm happy to help if needed
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org