Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Jan 6, 2021

What changes were proposed in this pull request?

This PR aims to add a basis for columnar encryption test framework by add OrcEncryptionSuite and FakeKeyProvider.

Please note that we will improve more in both Apache Spark and Apache ORC in Apache Spark 3.2.0 timeframe.

Why are the changes needed?

Apache ORC 1.6 supports columnar encryption.

Does this PR introduce any user-facing change?

No. This is for a test case.

How was this patch tested?

Pass the newly added test suite.

@dongjoon-hyun
Copy link
Member Author

Could you review this, @HyukjinKwon and @maropu ?

Copy link
Member

@maropu maropu left a comment

Choose a reason for hiding this comment

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

Minor comments and it looks okay otherwise (if the tests pass).

@dongjoon-hyun
Copy link
Member Author

Although only the last commit is the change, the PR is rebased to the master. Sorry about that.

@dongjoon-hyun
Copy link
Member Author

This is a test-only patch and tested like the following.

$ build/sbt "sql/testOnly *.OrcEncryptionSuite"
[info] OrcEncryptionSuite:
[info] - Write and read an encrypted file (3 seconds, 918 milliseconds)
[info] - Write and read an encrypted table (786 milliseconds)
...
[info] Suites: completed 1, aborted 0
[info] Tests: succeeded 2, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
[info] Passed: Total 2, Failed 0, Errors 0, Passed 2
[success] Total time: 77 s (01:17), completed Jan 6, 2021 5:19:14 AM

@dongjoon-hyun
Copy link
Member Author

Thank you for review and approval, @maropu !

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

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

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

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

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

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

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

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

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

Test build #133736 has finished for PR 31065 at commit 079998d.

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

@dongjoon-hyun
Copy link
Member Author

I'm looking at the Jenkins issue. It seems to fail due to environmental issue.

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

Test build #133743 has finished for PR 31065 at commit ff1f7d3.

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

@dongjoon-hyun
Copy link
Member Author

Retest this please

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

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

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

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

@SparkQA
Copy link

SparkQA commented Jan 6, 2021

Test build #133752 has finished for PR 31065 at commit ff1f7d3.

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

@dongjoon-hyun
Copy link
Member Author

The current Jenkins passed Java/Scala/Python UTs already.

Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-34029 branch January 6, 2021 21:00
@SparkQA
Copy link

SparkQA commented Jan 6, 2021

Test build #133754 has finished for PR 31065 at commit 070d3fd.

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

@dongjoon-hyun
Copy link
Member Author

Jenkins passed, but GitHub Action seems to hit another issue. I'll take a look.

@HyukjinKwon
Copy link
Member

+1 LGTM!

@HyukjinKwon
Copy link
Member

@dongjoon-hyun seems like it's still flaky (?). Can you take a quick look for https://github.com/apache/spark/pull/31068/checks?check_run_id=1660627329? I saw this in a PR not related to ORC change.

@dongjoon-hyun
Copy link
Member Author

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Jan 7, 2021

The Hadoop class is not loaded correctly in sql/core module and we cannot add test dependency.

The failure happens when we build first and run test separately. This is the similar one we hit before. I'm thinking about moving the test from sql/core to sql/hive

@HyukjinKwon
Copy link
Member

Thanks for working on it!

@dongjoon-hyun
Copy link
Member Author

Since the test suite was not isolated correctly, this is reverted via 194edc8

@dongjoon-hyun
Copy link
Member Author

According to the current observation, I'll make a new PR and make it pass GitHub Action and Jenkins Maven.

dongjoon-hyun added a commit that referenced this pull request Feb 21, 2021
### What changes were proposed in this pull request?

This is a retry of #31065 . Last time, the newly add test cases passed in Jenkins and individually, but it's reverted because they fail when `GitHub Action` runs with  `SERIAL_SBT_TESTS=1`.

In this PR, `SecurityTest` tag is used to isolate `KeyProvider`.

This PR aims to add a basis for columnar encryption test framework by add `OrcEncryptionSuite` and `FakeKeyProvider`.

Please note that we will improve more in both Apache Spark and Apache ORC in Apache Spark 3.2.0 timeframe.

### Why are the changes needed?

Apache ORC 1.6 supports columnar encryption.

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

No. This is for a test case.

### How was this patch tested?

Pass the newly added test suite.

Closes #31603 from dongjoon-hyun/SPARK-34486-RETRY.

Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants