aglinxinyuan commented on code in PR #6883:
URL: https://github.com/apache/texera/pull/6883#discussion_r3671323368
##########
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:
Fixed, and thanks — this was a real leak window. `tupleIterator =
firstTupleIterator` now happens immediately after `document.getRange(0, 1)` is
created, before it is probed, so a throw during hasNext/next()/JSON conversion
still hits the catch-path drain. On the other two flagged lines: `getRange`
appears only twice in this method, and the later `tupleIterator =
document.getRange(1, totalCount)` is safe because the first read is already
drained to its bound by then — so no further assignment was needed. Verified
with WorkflowExecutionService/Test/compile + scalafmt/scalafix.
--
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]
