Copilot commented on code in PR #6883:
URL: https://github.com/apache/texera/pull/6883#discussion_r3649369218
##########
amber/src/main/scala/org/apache/texera/web/resource/SyncExecutionResource.scala:
##########
@@ -540,9 +544,17 @@ class SyncExecutionResource extends LazyLogging {
val totalCount = document.getCount.toInt
val mapper = new ObjectMapper()
- val tupleIterator = document.get()
-
- if (totalCount == 0 || !tupleIterator.hasNext) {
+ // Bounded reads (getRange) release their Parquet/S3 reader inside
the
+ // next() that serves their last record, so each read below is sized
+ // to what this method actually consumes. The unbounded get()
+ // previously used here outlived the early returns and was only
+ // reclaimed by the GC finalizer. The first tuple gets its own
+ // single-record read because the visualization and
+ // oversized-first-tuple branches return after consuming exactly one
+ // tuple.
+ val firstTupleIterator = document.getRange(0, 1)
+
+ if (totalCount == 0 || !firstTupleIterator.hasNext) {
Review Comment:
`tupleIterator` is hoisted so the catch block can drain a partially-consumed
bounded read, but it isn’t set until after the visualization/first-tuple logic.
If an exception is thrown while probing/reading `firstTupleIterator` (e.g.,
during `hasNext`/`next()`/JSON conversion), the catch drain won’t run and the
underlying reader can still be leaked. Assign `tupleIterator =
firstTupleIterator` immediately so the catch-path drain covers the first
bounded read too.
This issue also appears in the following locations of the same file:
- line 568
- line 603
--
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]
