You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "anishshri-db (via GitHub)" <gi...@apache.org> on 2025/03/05 07:51:53 UTC
[PR] [SPARK-51397] Fix maintenance pool shutdown handling issue causing long test times [spark]
anishshri-db opened a new pull request, #50168:
URL: https://github.com/apache/spark/pull/50168
### What changes were proposed in this pull request?
Fix maintenance pool shutdown handling issue causing long test times
### Why are the changes needed?
Some of the snapshot lag verification tests were taking a long time.
Before the change: > 6m
After the change: ~21s
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing unit tests
### 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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times [spark]
Posted by "anishshri-db (via GitHub)" <gi...@apache.org>.
anishshri-db commented on PR #50168:
URL: https://github.com/apache/spark/pull/50168#issuecomment-2700138321
@HeartSaVioR - PTAL, thx !
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR closed pull request #50168: [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times
URL: https://github.com/apache/spark/pull/50168
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times [spark]
Posted by "anishshri-db (via GitHub)" <gi...@apache.org>.
anishshri-db commented on code in PR #50168:
URL: https://github.com/apache/spark/pull/50168#discussion_r1993203757
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala:
##########
@@ -851,14 +853,16 @@ object StateStore extends Logging {
threadPool.shutdown() // Disable new tasks from being submitted
// Wait a while for existing tasks to terminate
- if (!threadPool.awaitTermination(5 * 60, TimeUnit.SECONDS)) {
+ val waitTimeout = 5 * shutdownTimeout
Review Comment:
Done - updated
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times [spark]
Posted by "anishshri-db (via GitHub)" <gi...@apache.org>.
anishshri-db commented on PR #50168:
URL: https://github.com/apache/spark/pull/50168#issuecomment-2720722448
> Looks like the config we are adding is confusing since user can't reason about 5x of timeout they have set.
@HeartSaVioR - Updated the PR. PTAL, thanks !
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #50168:
URL: https://github.com/apache/spark/pull/50168#issuecomment-2723487378
CI has passed. Thanks! Merging to master.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51397][SS] Fix maintenance pool shutdown handling issue causing long test times [spark]
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on code in PR #50168:
URL: https://github.com/apache/spark/pull/50168#discussion_r1988509128
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala:
##########
@@ -851,14 +853,16 @@ object StateStore extends Logging {
threadPool.shutdown() // Disable new tasks from being submitted
// Wait a while for existing tasks to terminate
- if (!threadPool.awaitTermination(5 * 60, TimeUnit.SECONDS)) {
+ val waitTimeout = 5 * shutdownTimeout
Review Comment:
Wait, since the config is control the timeout to wait for shutdown, shouldn't the config refer to the `waitTimeout` here?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org