Skip to content

[SPARK-36682][CORE][TEST] Add Hadoop sequence file test for different Hadoop codecs #33924

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[SPARK-36682][CORE][TEST] Add Hadoop sequence file test for different Hadoop codecs #33924

wants to merge 3 commits into from

Conversation

@viirya
Copy link
Member

@viirya viirya commented Sep 7, 2021

What changes were proposed in this pull request?

This patch proposes to add e2e tests for using Hadoop codecs to write sequence files.

Why are the changes needed?

To improve test coverage.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added tests.

@github-actions github-actions bot added the CORE label Sep 7, 2021
@viirya viirya mentioned this pull request Sep 7, 2021
Closed
viirya
viirya commented Sep 7, 2021
View reviewed changes
core/src/test/scala/org/apache/spark/FileSuite.scala Outdated
Comment on lines 140 to 141
Seq((new DefaultCodec(), "default"), (new BZip2Codec(), "bzip2"), (new GzipCodec(), "gzip"),
(new Lz4Codec(), "lz4"), (new SnappyCodec, "snappy")).foreach { case (codec, codecName) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

lz4 codecs currently fails due to SPARK-36669.
snappy codec currently fails due to SPARK-36681.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for filing JIRAs.

@SparkQA
Copy link

SparkQA commented Sep 7, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47543/

@SparkQA
Copy link

SparkQA commented Sep 7, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47543/

@SparkQA
Copy link

SparkQA commented Sep 7, 2021

Test build #143040 has finished for PR 33924 at commit a0b115f.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member Author

viirya commented Sep 7, 2021

We have dedicated JIRAs for snappy and lz4 issues. We already verified the issues in above CI test, going to remove them first to make test passed.

@SparkQA
Copy link

SparkQA commented Sep 7, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47557/

@viirya
Copy link
Member Author

viirya commented Sep 7, 2021

cc @sunchao @cloud-fan @dbtsai too

dbtsai
dbtsai approved these changes Sep 7, 2021
View reviewed changes
Copy link
Member

@dbtsai dbtsai left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@SparkQA
Copy link

SparkQA commented Sep 7, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/47557/

sunchao
sunchao approved these changes Sep 7, 2021
View reviewed changes
Copy link
Member

@sunchao sunchao left a comment

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented Sep 7, 2021

Test build #143054 has finished for PR 33924 at commit a9139b3.

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

dbtsai
dbtsai reviewed Sep 7, 2021
View reviewed changes
core/src/test/scala/org/apache/spark/FileSuite.scala Outdated
}
}

// Hadoop "gzip" codec doesn't support sequence file yet.
Copy link
Member

Choose a reason for hiding this comment

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

Hadoop "gzip" codec requires native library installed for gzip compressed seq file.

dbtsai
dbtsai reviewed Sep 7, 2021
View reviewed changes
core/src/test/scala/org/apache/spark/FileSuite.scala Outdated
}

// Hadoop "gzip" codec doesn't support sequence file yet.
// Hadoop "zstd" codec needs native library installed.
Copy link
Member

Choose a reason for hiding this comment

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

zstd is the same as gzip before your gzip codec is released in hadoop. Maybe just say

Hadoop "gzip" and "zstd" codecs requires native library installed for sequence files

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. Revised as suggested.

@viirya
Copy link
Member Author

viirya commented Sep 7, 2021

The last commit is comment doc only. Thanks for reviewing. Merging to master/3.2.

@viirya viirya closed this in 6745d77 Sep 7, 2021
viirya added a commit that referenced this pull request Sep 7, 2021
@viirya
[SPARK-36682][CORE][TEST] Add Hadoop sequence file test for different…
acd3856 
… Hadoop codecs

### What changes were proposed in this pull request?

This patch proposes to add e2e tests for using Hadoop codecs to write sequence files.

### Why are the changes needed?

To improve test coverage.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Added tests.

Closes #33924 from viirya/hadoop-seq-test.

Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: Liang-Chi Hsieh <[email protected]>
(cherry picked from commit 6745d77)
Signed-off-by: Liang-Chi Hsieh <[email protected]>
@viirya viirya deleted the hadoop-seq-test branch September 7, 2021 20:20
@dongjoon-hyun
Copy link
Member

+1, late LGTM.

@HyukjinKwon
Copy link
Member

LGTM2

@cloud-fan
Copy link
Contributor

late LGTM

flyrain pushed a commit to flyrain/spark that referenced this pull request Sep 21, 2021
@viirya
[SPARK-36682][CORE][TEST] Add Hadoop sequence file test for different…
5cf7fd6 
… Hadoop codecs

### What changes were proposed in this pull request?

This patch proposes to add e2e tests for using Hadoop codecs to write sequence files.

### Why are the changes needed?

To improve test coverage.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Added tests.

Closes apache#33924 from viirya/hadoop-seq-test.

Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: Liang-Chi Hsieh <[email protected]>
(cherry picked from commit 6745d77)
Signed-off-by: Liang-Chi Hsieh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dbtsai dbtsai dbtsai approved these changes

@sunchao sunchao sunchao approved these changes

@dongjoon-hyun dongjoon-hyun Awaiting requested review from dongjoon-hyun

Assignees

No one assigned

Labels

CORE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@viirya @SparkQA @dongjoon-hyun @HyukjinKwon @cloud-fan @sunchao @dbtsai