peter-toth commented on code in PR #59639:
URL: https://github.com/apache/spark/pull/59639#discussion_r4339162291
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/LateMaterializationParquetRecordReader.java:
##########
@@ -730,12 +800,18 @@ private RowRanges evaluateStorageFilter(
splicedBytes = 0L;
}
}
- if (splicedBytes + rangeBytes > cap) {
+ if (splicedBytes + rangeCharge > cap) {
// The buffer goes first, which is a no-op once splicing is given up.
abandonSplicing();
splicedBytes = 0L;
// The ranges would be thrown away, so stop.
- if (rangeBytes > cap) return null;
+ if (rangeCharge > cap) return null;
+ }
+ if (!reserveSurvivorMemory((accountKeys ? splicedBytes : 0L) +
rangeCharge)) {
Review Comment:
Done in
[778c14954a9](https://github.com/apache/spark/pull/59639/commits/778c14954a9ede1af0fc6357b0b5546322e69a23).
Every reservation now rethrows through
`ParquetStorageFilter.memoryReservationFailed`:
- a `RuntimeException` goes out in a checked `SparkException`, as in
`throwIfKilled`;
- an `InternalError` goes out in an `UnknownError`.
A new test has another consumer's spill throw each one, and checks that
`shouldIgnoreCorruptFileException` rejects what comes out.
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/LateMaterializationParquetRecordReader.java:
##########
@@ -513,6 +568,16 @@ protected boolean loadNextRowGroup() throws IOException {
continue;
}
+ // A spill has the rest of a spliced row group read its keys again, in
part. That needs their
+ // offset indexes. Parquet builds a block's store once, over the columns
requested then, so it
+ // is built here over every projected column. Only a row group phase 2
reads in part can be
+ // spilled. Phase 2 reads its store anyway, and gives the filter up on
an unreadable index.
+ spillableRowGroup =
Review Comment:
Gone with the spill in
[778c14954a9](https://github.com/apache/spark/pull/59639/commits/778c14954a9ede1af0fc6357b0b5546322e69a23).
##########
sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/LateMaterializationParquetRecordReader.java:
##########
@@ -460,6 +507,11 @@ protected boolean loadNextRowGroup() throws IOException {
// assignment comes only after the next row group's phase 1, whose budget
does not count them.
closeDataPages();
releaseRowGroupReaders();
+ loadedRanges = null;
+ spillableRowGroup = false;
Review Comment:
Gone with the spill in
[778c14954a9](https://github.com/apache/spark/pull/59639/commits/778c14954a9ede1af0fc6357b0b5546322e69a23).
--
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]
