You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "singhpk234 (via GitHub)" <gi...@apache.org> on 2025/05/07 01:37:09 UTC
[PR] Spark: Make maxRecordPerMicrobatch a soft limit [iceberg]
singhpk234 opened a new pull request, #12988:
URL: https://github.com/apache/iceberg/pull/12988
### About the change
Make _maxRecordsPerMicrobatch_ a soft limit, as the cases like for ex number of records is less than the maxRecords of a file would expect us to read the file partially, this can't be just a row group boundary or something which we can incorporate in our scan tasks as if like splitting it at the record count cutoff, hence its very difficult to define the boundary, it would be better if we make the make this a soft limit as if when including a file if it can be contained within the limit its fine, otherwise include the whole file and be done with that particular microbatch stream.
This change is motivated by two major factors :
1. stream being stuck presently leading to poor UX https://github.com/apache/iceberg/pull/12217#discussion_r1962211721
2. Softlimit is what other solution enforce for ex : delta [doc](https://docs.databricks.com/aws/en/structured-streaming/delta-lake)
**_maxBytesPerTrigger: How much data gets processed in each micro-batch. This option sets a “soft max”, meaning that a batch processes approximately this amount of data and may process more than the limit in order to make the streaming query move forward in cases when the smallest input unit is larger than this limit. This is not set by default._**
### Testing done
Modified the existing UT which mimics stuckness to pass 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: 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: Make maxRecordPerMicrobatch a soft limit [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #12988:
URL: https://github.com/apache/iceberg/pull/12988#issuecomment-2856852701
seems un-related failure
```
TestRewriteDataFilesAction > testParallelPartialProgressWithMaxFailedCommitsLargerThanTotalFileGroup() > formatVersion = 2 FAILED
java.lang.RuntimeException: partial-progress.enabled is true but 1 rewrite commits failed. This is more than the maximum allowed failures of 0. Check the logs to determine why the individual commits failed. If this is persistent it may help to increase partial-progress.max-commits which will split the rewrite operation into smaller commits.
at org.apache.iceberg.spark.actions.RewriteDataFilesSparkAction.doExecuteWithPartialProgress(RewriteDataFilesSparkAction.java:400)
at org.apache.iceberg.spark.actions.RewriteDataFilesSparkAction.execute(RewriteDataFilesSparkAction.java:187)
```
--
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: Make maxRecordPerMicrobatch a soft limit [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #12988:
URL: https://github.com/apache/iceberg/pull/12988#discussion_r2162658679
##########
spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestStructuredStreamingRead3.java:
##########
@@ -200,7 +198,7 @@ public void testReadStreamWithMaxRows2() throws Exception {
assertThat(
microBatchCount(
ImmutableMap.of(SparkReadOptions.STREAMING_MAX_ROWS_PER_MICRO_BATCH, "2")))
- .isEqualTo(4);
Review Comment:
why is the micro batch count 4 previously?
##########
docs/docs/spark-configuration.md:
##########
@@ -225,8 +225,7 @@ spark.read
| streaming-max-rows-per-micro-batch | INT_MAX | Maximum number of rows per microbatch |
!!! warning
- streaming-max-rows-per-micro-batch should always be greater than the number of records in any data file in the table.
- The smallest unit that will be streamed is a single file, so if a data file contains more records than this limit, the stream will get stuck at this file.
+ streaming-max-rows-per-micro-batch option sets a “soft max”, a batch will always include all the rows in the next unprocessed data file but additional files will not be included if doing so would exceed the soft-max.
Review Comment:
nit: `soft-max` -> `soft limit` at the end of the sentence
--
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: Make maxRecordPerMicrobatch a soft limit [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 closed pull request #12988: Spark: Make maxRecordPerMicrobatch a soft limit
URL: https://github.com/apache/iceberg/pull/12988
--
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: Make maxRecordPerMicrobatch a soft limit [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #12988:
URL: https://github.com/apache/iceberg/pull/12988#discussion_r2153053702
##########
docs/docs/spark-configuration.md:
##########
@@ -225,8 +225,7 @@ spark.read
| streaming-max-rows-per-micro-batch | INT_MAX | Maximum number of rows per microbatch |
!!! warning
- streaming-max-rows-per-micro-batch should always be greater than the number of records in any data file in the table.
- The smallest unit that will be streamed is a single file, so if a data file contains more records than this limit, the stream will get stuck at this file.
+ streaming-max-rows-per-micro-batch option sets a “soft max”, meaning that a batch processes approximately this amount of records and may process more than the limit in order to make the streaming query move forward in cases when the smallest input unit is larger than this limit.
Review Comment:
This is a really complicated sentence, suggestion on a rewording?
```suggestion
streaming-max-rows-per-micro-batch option sets a “soft max”, a batch will always include all the rows in the next unprocessed data file but additional files will not be included if doing so would exceed the the soft-max.
```
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java:
##########
@@ -387,7 +387,7 @@ public Offset latestOffset(Offset startOffset, ReadLimit limit) {
boolean shouldContinueReading = true;
int curFilesAdded = 0;
- int curRecordCount = 0;
+ long curRecordCount = 0;
Review Comment:
Interesting ... I think think we may actually have a spark issue with we exceed int-max rows in a microbatch .... Not sure. I'm mostly thinking about the limits on array length in the inmemory versions of rows getting shipped to shufflers ... not important though. I'm just rambling
--
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: Make maxRecordPerMicrobatch a soft limit [iceberg]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #12988:
URL: https://github.com/apache/iceberg/pull/12988#discussion_r2158110714
##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkMicroBatchStream.java:
##########
@@ -387,7 +387,7 @@ public Offset latestOffset(Offset startOffset, ReadLimit limit) {
boolean shouldContinueReading = true;
int curFilesAdded = 0;
- int curRecordCount = 0;
+ long curRecordCount = 0;
Review Comment:
Ack, i just changed it to long as row count of a file is long and IJ as complaining !
totally agree if we were to actually have a file of this big size we might have trouble.
--
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