[GitHub] [spark] xiongbo-sjtu commented on pull request #43021: [SPARK-45227][CORE] Fix a subtle thread-safety issue with CoarseGrainedExecutorBackend
xiongbo-sjtu commented on PR #43021: URL: https://github.com/apache/spark/pull/43021#issuecomment-1740398612 Thanks. Just submit another pull request to patch branch-3.3 https://github.com/apache/spark/pull/43176 -- 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
[GitHub] [spark] xiongbo-sjtu commented on pull request #43021: [SPARK-45227][CORE] Fix a subtle thread-safety issue with CoarseGrainedExecutorBackend
xiongbo-sjtu commented on PR #43021: URL: https://github.com/apache/spark/pull/43021#issuecomment-1740112354 > @HeartSaVioR, @HyukjinKwon I am seeing multiple PR's failing [with this](https://github.com/xiongbo-sjtu/spark/actions/runs/6332587036/job/17203381073#step:10:24799). It looks like some timeout issue, did anything change recently which might be triggering it ? > > This PR should not be impacted by those modules, but I would have prefer clean builds before merging. All failed tests have passed after I rerun them. Solving the fragile unit tests is an orthogonal concern. Any more concern or blocker for merging this pull request? @mridulm -- 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
[GitHub] [spark] xiongbo-sjtu commented on pull request #43021: [SPARK-45227][CORE] Fix a subtle thread-safety issue with CoarseGrainedExecutorBackend
xiongbo-sjtu commented on PR #43021: URL: https://github.com/apache/spark/pull/43021#issuecomment-1736717071 Gentlemen, please 👍 if you're okay with getting this pull request merged. -- 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
[GitHub] [spark] xiongbo-sjtu commented on pull request #43021: [SPARK-45227][CORE] Fix a subtle thread-safety issue with CoarseGrainedExecutorBackend
xiongbo-sjtu commented on PR #43021: URL: https://github.com/apache/spark/pull/43021#issuecomment-1736610019 > I think it would be good to add the content of [SPARK-45227](https://issues.apache.org/jira/browse/SPARK-45227) here to the PR's description. -- 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
[GitHub] [spark] xiongbo-sjtu commented on pull request #43021: [SPARK-45227][CORE] Fix a subtle thread-safety issue with CoarseGrainedExecutorBackend
xiongbo-sjtu commented on PR #43021: URL: https://github.com/apache/spark/pull/43021#issuecomment-1736478403 +1 to the comment made by @JoshRosen Our production fleet has tens of thousands Spark jobs running daily. After migrating those jobs from Spark 2 to Spark 3, we've observed the issue (i.e., thread dumps show that the dispatcher thread is stuck with the reported stack trace) only a handful of times. The unit test was added so that the thread-safety will not be unintentionally refactored away later. It's not meant to simulate the race condition. The bottom line is that no functional regression is expected after this pull request replaces the vanilla mutable.hashmap with CHM. -- 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
[GitHub] [spark] xiongbo-sjtu commented on pull request #43021: [SPARK-45227][CORE] Fix a subtle thread-safety issue with CoarseGrainedExecutorBackend
xiongbo-sjtu commented on PR #43021: URL: https://github.com/apache/spark/pull/43021#issuecomment-1732099260 @jiangxb1987 @mridulm Eventually got all tests passed in Github Actions. Any concern on merging this pull request? As a side note, I've discovered [another minor issue](https://issues.apache.org/jira/browse/SPARK-45283), but will address that in another pull request. Thanks, Bo -- 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