You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by djvulee <gi...@git.apache.org> on 2016/09/11 19:54:00 UTC
[GitHub] spark pull request #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metri...
GitHub user djvulee opened a pull request:
https://github.com/apache/spark/pull/15052
[SPARK-17500][PySpark]Make DiskBytesSpilled metric in PySpark shuffle right
## What changes were proposed in this pull request?
The origin way increases the DiskBytesSpilled metric with the file size during each spill in ExternalMerger && ExternalGroupBy, but we only need the last size.
## How was this patch tested?
No extra tests, because this just update the metrics
Author: Li Hu <li...@bytedance.com>
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/djvulee/spark PyDiskSpillMetric
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/15052.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #15052
----
commit 1b90b0dd61c22ffba6d578f73cf5aca88629b1be
Author: DjvuLee <li...@bytedance.com>
Date: 2016-09-11T19:41:32Z
Make DiskBytesSpilled metric in PySpark shuffle right
The origin way increase the DiskBytesSpilled metric with the file
size during each spill in ExternalMerger && ExternalGroupBy, but we only need the last size.
No extra Tests, because this just update the metrics
Author: Li Hu <li...@bytedance.com>
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by djvulee <gi...@git.apache.org>.
Github user djvulee commented on the issue:
https://github.com/apache/spark/pull/15052
@srowen @davies mind taking a look? This PR is very simple.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/15052
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metri...
Posted by djvulee <gi...@git.apache.org>.
Github user djvulee closed the pull request at:
https://github.com/apache/spark/pull/15052
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by djvulee <gi...@git.apache.org>.
Github user djvulee commented on the issue:
https://github.com/apache/spark/pull/15052
@srowen Yes, the file seems always empty before write, so the origin way is OK. Sorry for this PR is not thoughtful enough, I just get a mislead by the other method in the shuffle.py, which used the pos before write.
I will close this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/15052
Given how DiskBytesSpilled is used, and still used in other parts of the code, this doesn't look correct. It seems to be a global that is always incremented. Here you reset the value in certain cases, effectively.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by djvulee <gi...@git.apache.org>.
Github user djvulee commented on the issue:
https://github.com/apache/spark/pull/15052
@srowen I update PR using an increment way to update the DiskBytesSpilled metrics.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by djvulee <gi...@git.apache.org>.
Github user djvulee commented on the issue:
https://github.com/apache/spark/pull/15052
@srowen you are right, I will correct it soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/15052
OK well if you see evidence later that the disk spilled bytes are unreasonably high, it's worth reinvestigating to see if there's a problem like this. If you aren't seeing bad metrics though, then maybe there's no problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by djvulee <gi...@git.apache.org>.
Github user djvulee commented on the issue:
https://github.com/apache/spark/pull/15052
@srowen No. It does not matter whether the file is empty or not, if the file is empty, the `getsize()` just return 0, and this should be OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/15052
I get that, but if it's always true, then there was no problem to begin with. That's what the code seems to think right now. I haven't looked at the code much but that's the question -- are you sure the files are non-empty in some scenario?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #15052: [SPARK-17500][PySpark]Make DiskBytesSpilled metric in Py...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/15052
Is the idea that the file may be non empty when written ?
There is at least one more instance of this call but maybe the file is known to be empty before.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org