-
Notifications
You must be signed in to change notification settings - Fork 28k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DO-NOT-MERGE] Investigate 'WorkerMemoryTest.test_memory_limit' failure #27183
[DO-NOT-MERGE] Investigate 'WorkerMemoryTest.test_memory_limit' failure #27183
Conversation
Test build #116598 has finished for PR 27183 at commit
|
… on pyspark test"" This reverts commit d0983af.
Okay, seems it's hitting the OOM due to too small limit apparently:
|
Test build #116599 has finished for PR 27183 at commit
|
IMHO, we don't necessarily need to set it very smaller if that's the case. It just needs to be different from default value, and we'll never conflict with such huge default value |
I think we should set it high enough for now. |
Test build #116602 has finished for PR 27183 at commit
|
python/pyspark/tests/test_worker.py
Outdated
"Memory limit feature in Python worker is dependent on " | ||
"Python's 'resource' module; however, not found.") | ||
not has_resource_module or "pypy" in platform.python_implementation().lower(), | ||
resource_requirement_message or "This test does not pass on pypy. See SPARK-30480") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll drop Python 2 very soon and the current PyPy in Jenkins is compatible with Python 2. So, it won't be a big issue to skip the test for now.
Test build #116604 has finished for PR 27183 at commit
|
Test build #116606 has finished for PR 27183 at commit
|
Made a PR at #27186 |
What changes were proposed in this pull request?
WorkerMemoryTest. test_memory_limit
fails apparently on Jenkins for an unknown reason. This PR targets to investigate.Why are the changes needed?
TBD
Does this PR introduce any user-facing change?
TBD
How was this patch tested?
TBD