[SPARK-2469] Use Snappy (instead of LZF) for default shuffle compression codec

[rxin]

This reduces shuffle compression memory usage by 3x.

[SparkQA]

QA tests have started for PR 1415. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16665/consoleFull

[mridulm]

Do we want to change default for everything or only for shuffle ? (only shuffle wont impact anything outside of spark)
What would be impact on user data if we change for all ? (It is developerApi after all, so there might be user data consuming this ?)

[rxin]

This is actually only used in shuffle.

[rxin]

Actually I lied. Somebody else added some code to use the compression codec to compress event data ...

[rxin]

It's actually bad to use these compression codecs to compress event data, because there is no guarantee they can be decompressed on different platforms ...

[rxin]

cc @andrewor14 I guess you added the event code ...

[rxin]

I looked into the event logger code and it appears that codec change should be fine. It figures out the codec for old data automatically anyway.

[andrewor14]

Yes, we log the codec used in a separate file so we don't lock ourselves out of our old event logs. This change seems fine.

[mridulm]

@andrewor14 do we also log the block size, etc of the codec used ?
If yes, then atleast for event data we should be fine.

IIRC we use the codec to compress
a) RDD (which could be in tachyon - and so shared between spark deployments ?),
b) shuffle (private to spark),
c) broadcast (private to spark).
d) event logging (discussed above)
e) checkpoint (could be shared between runs ?)

Other than (a) and (e), sharing data via others would be non trivial and something we dont need to support imo.
I am not very sure of (a) and (e) - thoughts ?

[rxin]

We should create a JIRA so compression streams use the first few bytes to track the compression codec and various settings it needs (for lzf/snappy/lz4, there isn't any).

I think currently compressed blocks in Tachyon can be a problem, and e I'm less sure about. @tdas can you comment?

[SparkQA]

QA results for PR 1415:
- This patch FAILED unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16665/consoleFull

[mridulm]

weird that test failures - unrelated to this change

[mridulm]

ah yes, blocksize is only used during compression time : and inferred from stream during decompression.
Then only class name should be sufficient

[rxin]

Yea the test failure isn't related.

If there is no objection, I'm going to merge this tomorrow. I will file a jira ticket so we can prepend compression codec information to compressed data and then perhaps pick compression codec during decompression based on that.

[mridulm]

Cant comment on tachyon since we dont use it and have no experience with it unfortunately.
I am fine with this change for the rest.

[pwendell]

@rxin IIRC at one point we changed this before and it caused a performance regression for our perf suite so we reverted it. At the time I think we were running on smaller data sets though. Maybe in this case were are willing to take a hit?

[rxin]

Yea - stability seems much more important than a small performance gain ....

[vanzin]

Only the codec names are stored in the event logs; no other information is currently recorded. But this change isn't really breaking anything in that area. (And, by default, event logs are not compressed.)

[rxin]

FYI filed JIRA: https://issues.apache.org/jira/browse/SPARK-2496 Compression streams should write its codec info to the stream

[rxin]

Ok I'm merging this one. Thanks guys.