Re: [PR] [WIP][SPARK-47819][CONNECT][3.5] Use asynchronous callback for execution cleanup [spark]
dongjoon-hyun commented on PR #46064: URL: https://github.com/apache/spark/pull/46064#issuecomment-2057374103 > @dongjoon-hyun I updated the ticket. Thank you for updating. > I am a bit mystified by this though, this was never an issue before. This particular case can be both seen as a bug/improvement. I agree with you. We can choose our path. - If we want to have this in `branch-3.5`, we need to categorize it as a `Bug`. - Otherwise, we can keep it in `master` only as `Improvement` and wait and see the customer report at Apache Spark 4.0.0. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [WIP][SPARK-47819][CONNECT][3.5] Use asynchronous callback for execution cleanup [spark]
hvanhovell commented on PR #46064: URL: https://github.com/apache/spark/pull/46064#issuecomment-2057331063 @dongjoon-hyun I updated the ticket. I am a bit mystified by this though, this was never an issue before. This particular case can be both seen as a bug/improvement. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[PR] [WIP][SPARK-47819][CONNECT][3.5] Use asynchronous callback for execution cleanup [spark]
xi-db opened a new pull request, #46064: URL: https://github.com/apache/spark/pull/46064 ([Original PR](https://github.com/apache/spark/pull/46027)) ### What changes were proposed in this pull request? Expired sessions are regularly checked and cleaned up by a maintenance thread. However, currently, this process is synchronous. Therefore, in rare cases, interrupting the execution thread of a query in a session can take hours, causing the entire maintenance process to stall, resulting in a large amount of memory not being cleared. We address this by introducing asynchronous callbacks for execution cleanup, avoiding synchronous joins of execution threads, and preventing the maintenance thread from stalling in the above scenarios. To be more specific, instead of calling `runner.join()` in `ExecutorHolder.close()`, we set a post-cleanup function as the callback through `runner.processOnCompletion`, which will be called asynchronously once the execution runner is completed or interrupted. In this way, the maintenance thread won't get blocked on joining an execution thread. ### Why are the changes needed? In the rare cases mentioned above, performance can be severely affected. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests and a new test `Async cleanup callback gets called after the execution is closed` in `SparkConnectServiceE2ESuite.scala`. ### Was this patch authored or co-authored using generative AI tooling? No. -- 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: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
