You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/03/03 07:18:32 UTC
[GitHub] [spark] HyukjinKwon opened a new pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
HyukjinKwon opened a new pull request #31723:
URL: https://github.com/apache/spark/pull/31723
### What changes were proposed in this pull request?
`TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse` can be flaky and fails sometimes:
```
======================================================================
ERROR [1.798s]: test_task_context_correct_with_python_worker_reuse (pyspark.tests.test_taskcontext.TaskContextTestsWithWorkerReuse)
...
test_task_context_correct_with_python_worker_reuse
self.assertTrue(pid in worker_pids)
AssertionError: False is not true
----------------------------------------------------------------------
```
I suspect that the Python worker was killed for whatever reason and new attempt created a new Python worker.
Reusing Python workers actually sort of makes the best effort reuse the Python workers. For example, if a Python worker is failed, the task will be relaunched, and then the Python workers could have different process IDs.
This PR fixes the flakiness simply by retrying the test case.
### Why are the changes needed?
To make the tests more robust.
### Does this PR introduce _any_ user-facing change?
No, dev-only.
### How was this patch tested?
Manually tested it by controlling the conditions manually in the test codes.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31723:
URL: https://github.com/apache/spark/pull/31723#discussion_r586182458
##########
File path: python/pyspark/tests/test_taskcontext.py
##########
@@ -282,6 +281,11 @@ def context(iterator):
self.assertTrue(bps == (-1, -1))
for pid in pids:
self.assertTrue(pid in worker_pids)
+ return True
+
+ def test_task_context_correct_with_python_worker_reuse(self):
+ eventually(
+ self.check_task_context_correct_with_python_worker_reuse, catch_assertions=True)
Review comment:
thank you :)
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31723:
URL: https://github.com/apache/spark/pull/31723#issuecomment-789547519
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135698/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #31723:
URL: https://github.com/apache/spark/pull/31723
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31723:
URL: https://github.com/apache/spark/pull/31723#issuecomment-790154235
Merged to master, branch-3.1 and branch-3.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.
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
[GitHub] [spark] HyukjinKwon commented on pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #31723:
URL: https://github.com/apache/spark/pull/31723#issuecomment-789498088
cc @viirya would you mind taking a quick look 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #31723:
URL: https://github.com/apache/spark/pull/31723#discussion_r586178392
##########
File path: python/pyspark/tests/test_taskcontext.py
##########
@@ -282,6 +281,11 @@ def context(iterator):
self.assertTrue(bps == (-1, -1))
for pid in pids:
self.assertTrue(pid in worker_pids)
+ return True
+
+ def test_task_context_correct_with_python_worker_reuse(self):
+ eventually(
+ self.check_task_context_correct_with_python_worker_reuse, catch_assertions=True)
Review comment:
nit: we may add few comment like the one in the description, e.g. "Retrying the check as reusing Python workers actually sort of makes the best effort reuse the Python workers."
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31723:
URL: https://github.com/apache/spark/pull/31723#issuecomment-789522993
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40280/
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31723: [SPARK-34604][PYTHON][TESTS] Use eventually in TaskContextTestsWithWorkerReuse.test_task_context_correct_with_python_worker_reuse
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31723:
URL: https://github.com/apache/spark/pull/31723#discussion_r586178880
##########
File path: python/pyspark/tests/test_taskcontext.py
##########
@@ -282,6 +281,11 @@ def context(iterator):
self.assertTrue(bps == (-1, -1))
for pid in pids:
self.assertTrue(pid in worker_pids)
+ return True
+
+ def test_task_context_correct_with_python_worker_reuse(self):
+ eventually(
+ self.check_task_context_correct_with_python_worker_reuse, catch_assertions=True)
Review comment:
Sure sure 👌
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org