dongjoon-hyun commented on code in PR #59961:
URL: https://github.com/apache/spark/pull/59961#discussion_r4476439928
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ffi/NativeDataSource.scala:
##########
@@ -858,9 +897,21 @@ class NativePartitionReader(
override def close(): Unit = synchronized {
if (!closed) {
closed = true
- if (!reading) {
+ // Another thread may be in next, or still read the batch that next
returned to it.
+ if (!reading && consumerDone) {
Review Comment:
@gengliangwang, could you share where the Cleaner rework stands? On Nov 2,
you planned it for the next revision of this PR
([r4445266538](https://github.com/apache/spark/pull/59961#discussion_r4445266538),
[r4445873980](https://github.com/apache/spark/pull/59961#discussion_r4445873980)):
`close()` frees everything at once after the end of the stream, a Cleaner
frees the last batch's buffers and the child allocator only when `close()` runs
before the end, and a `next()` after `close()` fails. 3b96894 adds the path 2
fallback from that plan, but the release logic of `NativePartitionReader` has
not changed since 050cc61. So the reader still decides the release by the
thread that called `next()` last, and this P1 still holds at the current head.
On Nov 4, you called it a design question still open in this thread and raised
the GC trade-off for a reader closed before the end of its stream
([r4462281986](https://github.com/apache/spark/pull/59961#discussion_r4462281986)).
1. Do you still plan to do this rework in this PR, or in a follow-up?
2. Are you waiting for reviewers to decide on that GC trade-off before you
start?
3. If it goes to a follow-up, how would this P1 be handled until then?
--
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]
