[SPARK-21276][CORE] Update lz4-java to the latest (v1.4.0) #18883
Conversation
|
Test build #80394 has finished for PR 18883 at commit
|
|
I'll update soon. |
| * | ||
| * TODO: merge this into upstream | ||
| */ | ||
| public final class LZ4BlockInputStream extends FilterInputStream { |
There was a problem hiding this comment.
Yeah I guess this needs a MiMa exclude. It's technically public but nobody should have ever referenced this directly. The codec class is separate.
There was a problem hiding this comment.
yea, thanks! I'll check and update soon.
|
Test build #80395 has finished for PR 18883 at commit
|
|
Retest this please. |
|
Test build #80406 has finished for PR 18883 at commit
|
|
Retest this please. |
|
Test build #80412 has finished for PR 18883 at commit
|
|
@dongjoon-hyun Thanks for your test re-triggers! |
|
It's because I like your PR. :) |
|
LGTM |
|
Merged to master |
| ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.status.api.v1.ShuffleReadMetricDistributions.this"), | ||
|
|
||
| // [SPARK-21276] Update lz4-java to the latest (v1.4.0) | ||
| ProblemFilters.exclude[MissingClassProblem]("org.apache.spark.io.LZ4BlockInputStream") |
There was a problem hiding this comment.
@srowen This is a breaking change. We should not remove a public class that is in the api docs: http://spark.apache.org/docs/latest/api/scala/index.html#org.apache.spark.io.LZ4BlockInputStream
There was a problem hiding this comment.
It's "public" only insofar as it has to be in Java to use it this way. There's no case where a user should or would use this class directly.
There was a problem hiding this comment.
But the user may write some codes to run different logics according to the InputStream types.
There was a problem hiding this comment.
By the way, I'm not sure if we want to pursue strictly compatible. Just to point out the issue here.
What changes were proposed in this pull request?
This pr updated
lz4-javato the latest (v1.4.0) and removed customLZ4BlockInputStream. We currently use customLZ4BlockInputStreamto read concatenated byte stream in shuffle. But, this functionality has been implemented in the latest lz4-java (lz4/lz4-java#105). So, we might update the latest to remove the customLZ4BlockInputStream.Major diffs between the latest release and v1.3.0 in the master are as follows (lz4/lz4-java@62f7547...6d4693f);
How was this patch tested?
Existing tests.