anishshri-db commented on PR #42742:
URL: https://github.com/apache/spark/pull/42742#issuecomment-1701633031
> Instead, what do you think about replacing the
Left(unrollMemoryUsedByThisBlock) with a throw new InterruptedException()? That
exception will bubble up and help the task to exit sooner.
@JoshRosen - I was going to do exactly this initially :) But I was not sure
whether this would be entirely safe or not, in terms of disposing the byte
buffers.
It seems the caller relies on the result here to create
`PartiallySerializedBlock` in case of failure
And within this, we add a task completion listener to ensure that we call
dispose on the byte buffers ?
```
private lazy val unrolledBuffer: ChunkedByteBuffer = {
bbos.close()
bbos.toChunkedByteBuffer
}
// If the task does not fully consume `valuesIterator` or otherwise fails
to consume or dispose of
// this PartiallySerializedBlock then we risk leaking of direct buffers,
so we use a task
// completion listener here in order to ensure that `unrolled.dispose()`
is called at least once.
// The dispose() method is idempotent, so it's safe to call it
unconditionally.
Option(TaskContext.get()).foreach { taskContext =>
taskContext.addTaskCompletionListener[Unit] { _ =>
// When a task completes, its unroll memory will automatically be
freed. Thus we do not call
// releaseUnrollMemoryForThisTask() here because we want to avoid
double-freeing.
unrolledBuffer.dispose()
}
}
```
So it seems freeing the memory is not a problem, but if we return the
`InterruptedException`, we would still be risking leaking the direct buffers,
since we won't get a chance to register this task completion listener ? Do you
think this is safe/handled elsewhere in the caller when the exception is
received ?
--
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]
