Skip to content

[MINOR][BUILD][PYSPARK] Disable test_memory_limit test temporarily #27158

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.

Sign up for GitHub

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

Closed
HeartSaVioR wants to merge 2 commits into from
Closed

[MINOR][BUILD][PYSPARK] Disable test_memory_limit test temporarily #27158

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07608af
[MINOR][BUILD][PYSPARK] Disable test_memory_limit test temporarily
HeartSaVioR Jan 10, 2020
23ad40b
Revert pip8 violation (my bad)
HeartSaVioR Jan 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion python/pyspark/tests/test_worker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def test_reuse_worker_of_parallelize_xrange(self):
"Memory limit feature in Python worker is dependent on "
"Python's 'resource' module; however, not found.")
class WorkerMemoryTest(PySparkTestCase):

@unittest.skip("disabled temporarily since it's failing consistently")
def test_memory_limit(self):
self.sc._conf.set("spark.executor.pyspark.memory", "1m")
Copy link
Member

@srowen srowen Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other thing I can think of is that this toy memory limit is too low. What if it were 8m or something? if you feel like it you could try testing that too

Copy link
Contributor Author

@HeartSaVioR HeartSaVioR Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The odd thing is that it passes with local dev. (MacOS, python3 - 3.7.4/python2 - 2.7.10/pypy - Python 2.7.13 & pypy 7.3.0 GCC 4.2.1) so not clear it would help, but if we suspect it we can try.

Copy link
Member

@HyukjinKwon HyukjinKwon Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will pass on Mac OS because Mac doesn't actually limits the memory. it doesn't work on Windows either. So, this hypothesis could make sense. Can we increase to something like 8m?

Copy link
Contributor Author

@HeartSaVioR HeartSaVioR Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK let me do it instead. Let's modify the PR title/description after confirming it works.

Copy link
Member

@HyukjinKwon HyukjinKwon Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's best to file a JIRA btw.

Copy link
Contributor Author

@HeartSaVioR HeartSaVioR Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about filing an issue after confirming the trick doesn't work and we have to disable? We may be able to just treat this PR as MINOR but mentioning the new JIRA issue in "reason" which is expected to address this.

Copy link
Member

@HyukjinKwon HyukjinKwon Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that works too. FWIW, I doubt if it consistently fails in the master yet. If that's the case, it should better fix the test here.

Copy link
Contributor Author

@HeartSaVioR HeartSaVioR Jan 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI #27159 is the patch increasing the value to 8m

rdd = self.sc.parallelize(xrange(1), 1)
Expand Down