[GitHub] [spark] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-714922479 thanks all!! 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-714495723 retest this please 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-714181079 @holdenk ? 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-711607829 @holdenk take a look? 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-708379134 kindly ping @holdenk 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-706718702 cc @holdenk for taking another look. 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-706235848 @holdenk Sorry for the delay. I had taken some days off recently. I'm addressing your comments right now and debugging the integration test(the decommissioned executor doesn't migrate shuffle blocks as expected, so I'm still investigating it.) Thank you for the further review :) 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-699868395 @holdenk Thanks for the review! I've addressed your most comments. And tests are updated and added in fa04b493324ea52698bed7ce15795530052b37be and 9d0f36d73e5750267ec2f33e7d964fc09778767f . 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-698080541 > That being said I still have concerns this PR is not sufficiently tested, can you add some more tests for the new flows you've introduced? There's only one new flow that is from Master to Worker. I can update the existing test by verifying Worker's decommission status... What kind of other concerns do you have? Could you elaborate more? So I can improve the PR accordingly. 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-698080541 > That being said I still have concerns this PR is not sufficiently tested, can you add some more tests for the new flows you've introduced? There's only one new flow that is from Master to Worker. I can update the existing test by verifying Worker's decommission status... What kind of other concerns do you have? Could you elaborate more? So I can improve the PR accordingly. 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-698071835 > This PR in its original broke another one of the conditions. Except for the breaking of the log(like you just fixed), what other conditions this PR breaks? 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-696469625 > I think we should not commit this with the K8s test being broken. We don't. That's also why I added `[K8S]` tag in the PR title. And feel free to leave comments, I can address them in followups. 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-696100181 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-696469625 > I think we should not commit this with the K8s test being broken. We don't. That's also why I added `[K8S]` tag in the PR title. And feel free to leave comments, I can address them in followups. 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. 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] Ngone51 commented on pull request #29817: [SPARK-32850][CORE][K8S] Simplify the RPC message flow of decommission
Ngone51 commented on pull request #29817: URL: https://github.com/apache/spark/pull/29817#issuecomment-696100181 Bring this back since it isn't the original commit that breaks the K8S test. As the PR https://github.com/apache/spark/pull/29751 merged before this already failed the K8S tests 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. 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