Skip to content
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

[SPARK-20177]Document about compression way has some little detail ch… #17498

Closed
wants to merge 2 commits into from
Closed

[SPARK-20177]Document about compression way has some little detail ch… #17498

wants to merge 2 commits into from

Conversation

guoxiaolongzte
Copy link

@guoxiaolongzte guoxiaolongzte commented Mar 31, 2017

…anges.

What changes were proposed in this pull request?

Document compression way little detail changes.
1.spark.eventLog.compress add 'Compression will use spark.io.compression.codec.'
2.spark.broadcast.compress add 'Compression will use spark.io.compression.codec.'
3,spark.rdd.compress add 'Compression will use spark.io.compression.codec.'
4.spark.io.compression.codec add 'event log describe'.

eg
Through the documents, I don't know what is compression mode about 'event log'.

How was this patch tested?

manual tests

Please review http://spark.apache.org/contributing.html before opening a pull request.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

I think that's OK otherwise

</td>
</tr>
<tr>
<td><code>spark.io.compression.codec</code></td>
<td>lz4</td>
<td>
The codec used to compress internal data such as RDD partitions, broadcast variables and
shuffle outputs. By default, Spark provides three codecs: <code>lz4</code>, <code>lzf</code>,
The codec used to compress internal data such as RDD partitions,event log, broadcast variables
Copy link
Member

Choose a reason for hiding this comment

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

(missing a space)

Copy link
Author

Choose a reason for hiding this comment

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

I asked a few people, they all don't know, the event log compression methods is spark.io.com pression. Codec.And a few other places it is necessary to modify, more friendly and clear.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what you're replying to, but I'm asking you to add the missing space in "partitions,event"

@jerryshao
Copy link
Contributor

jerryshao commented Apr 1, 2017

IMHO I thought this is still a not so necessary fix. I would doubt if user really get confused without your fix? You can always correct me since I stand on the of developers :).

@guoxiaolongzte
Copy link
Author

@srowen i add a space

@guoxiaolongzte
Copy link
Author

@jerryshao
This is just an optimization suggestion.

@guoxiaolongzte
Copy link
Author

@srowen @jerryshao
If a spark application developer, using event compress, from the spark official document, will not see the use of spark.io.compression.codec is specified compression description.
The current problem I think is how to change the document.
I mentioned above to amend the point, which can be a few changes to merge to the master

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

It's borderline too trivial to add, yes, but I think it's an OK doc improvement.

@SparkQA
Copy link

SparkQA commented Apr 1, 2017

Test build #3632 has finished for PR 17498 at commit 3059013.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Apr 1, 2017

Merged to master

@asfgit asfgit closed this in cf5963c Apr 1, 2017
@guoxiaolongzte guoxiaolongzte deleted the SPARK-20177 branch June 12, 2017 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants