Skip to content

[SPARK-37282][TESTS] Add ExtendedLevelDBTest and disable LevelDB tests on Apple Silicon#34548

Closed
dongjoon-hyun wants to merge 1 commit intoapache:masterfrom
dongjoon-hyun:SPARK-37282
Closed

[SPARK-37282][TESTS] Add ExtendedLevelDBTest and disable LevelDB tests on Apple Silicon#34548
dongjoon-hyun wants to merge 1 commit intoapache:masterfrom
dongjoon-hyun:SPARK-37282

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Nov 11, 2021

What changes were proposed in this pull request?

This PR aims to add ExtendedLevelDBTest to disable LevelDB selectively on Apple Silicon.

  • Add ExtendedLevelDBTest test tag
  • Disable LevelDB test cases from kvstore module/HistoryServerSuite/AppStatusStoreSuite.

Why are the changes needed?

Java 17 officially support Apple Silicon.

In addition, Oracle Java, Azul Zulu, and Eclipse Temurin Java 17 supports Apple Silicon natively.

/Users/dongjoon/.jenv/versions/oracle17/bin/java: Mach-O 64-bit executable arm64
/Users/dongjoon/.jenv/versions/zulu17/bin/java: Mach-O 64-bit executable arm64
/Users/dongjoon/.jenv/versions/temurin17/bin/java: Mach-O 64-bit executable arm64

However, LevelDBJNI still doesn't support Apple Silicon natively, the relevant test cases fail on M1.

Does this PR introduce any user-facing change?

No. This is a test-only PR.

How was this patch tested?

Run the above tests on M1.


@Override
protected KVStore createStore() throws Exception {
assumeFalse(SystemUtils.IS_OS_MAC_OSX && System.getProperty("os.arch").equals("aarch64"));
Copy link
Member Author

Choose a reason for hiding this comment

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

We cannot use ExtendedLevelDBTest in Java test.


@Before
public void setup() throws Exception {
assumeFalse(SystemUtils.IS_OS_MAC_OSX && System.getProperty("os.arch").equals("aarch64"));
Copy link
Member Author

Choose a reason for hiding this comment

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

We cannot use ExtendedLevelDBTest in Java test.


if (!inMemory) {
// LevelDB doesn't support Apple Silicon yet
assume(!(Utils.isMac && System.getProperty("os.arch").equals("aarch64")))
Copy link
Member Author

Choose a reason for hiding this comment

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

This test case will run in-memory test still on M1.

// RocksDB doesn't support Apple Silicon yet
if (System.getProperty("os.name").equals("Mac OS X") &&
System.getProperty("os.arch").equals("aarch64")) {
if (Utils.isMac && System.getProperty("os.arch").equals("aarch64")) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I piggy-back this replacement from System.getProperty("os.name").equals("Mac OS X") to Utils.isMac.

@dongjoon-hyun
Copy link
Member Author

Thank you, @HyukjinKwon .

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

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

@dongjoon-hyun
Copy link
Member Author

Merged to master.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-37282 branch November 11, 2021 07:03
@SparkQA
Copy link

SparkQA commented Nov 11, 2021

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

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

Test build #145083 has finished for PR 34548 at commit 3c3605e.

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

sarutak pushed a commit that referenced this pull request Nov 13, 2021
…ndedLevelDBTest

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

This PR is a follow-up of #34548. This is missed due to `-Pyarn` profile.

### Why are the changes needed?

This is required to pass `yarn` module on Apple Silicon.

**BEFORE**
```
$ build/sbt "yarn/test"
...
[info] YarnShuffleServiceSuite:
[info] org.apache.spark.network.yarn.YarnShuffleServiceSuite *** ABORTED *** (20 milliseconds)
[info]   java.lang.UnsatisfiedLinkError: Could not load library. Reasons: [no leveldbjni64-1.8
...
```

**AFTER**
```
$ build/sbt "yarn/test" -Pyarn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedLevelDBTest
...
[info] Run completed in 4 minutes, 57 seconds.
[info] Total number of tests run: 135
[info] Suites: completed 18, aborted 0
[info] Tests: succeeded 135, failed 0, canceled 1, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 319 s (05:19), completed Nov 12, 2021, 4:53:14 PM
```

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

No.

### How was this patch tested?

A manual test on Apple Silicon.
```
$ build/sbt "yarn/test" -Pyarn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedLevelDBTest
```

Closes #34576 from dongjoon-hyun/SPARK-37282-2.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.com>
@LuciferYang
Copy link
Contributor

Should we extract SystemUtils.IS_OS_MAC_OSX && SystemUtils.OS_ARCH.equals("aarch64") into an independent val, such as Utils.isAppleSilicon @dongjoon-hyun

@HyukjinKwon
Copy link
Member

SGTM

@LuciferYang
Copy link
Contributor

LuciferYang commented Nov 18, 2021

SGTM

Let me do this later

sunchao pushed a commit to sunchao/spark that referenced this pull request Dec 8, 2021
…sts on Apple Silicon

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

This PR aims to add `ExtendedLevelDBTest ` to disable `LevelDB` selectively on Apple Silicon.
- Add `ExtendedLevelDBTest ` test tag
- Disable LevelDB test cases from `kvstore` module/`HistoryServerSuite`/`AppStatusStoreSuite`.

### Why are the changes needed?

Java 17 officially support Apple Silicon.
- JEP 391: macOS/AArch64 Port
- https://bugs.openjdk.java.net/browse/JDK-8251280

In addition, Oracle Java, Azul Zulu, and Eclipse Temurin Java 17 supports Apple Silicon natively.
```
/Users/dongjoon/.jenv/versions/oracle17/bin/java: Mach-O 64-bit executable arm64
/Users/dongjoon/.jenv/versions/zulu17/bin/java: Mach-O 64-bit executable arm64
/Users/dongjoon/.jenv/versions/temurin17/bin/java: Mach-O 64-bit executable arm64
```

However, LevelDBJNI still doesn't support Apple Silicon natively, the relevant test cases fail on M1.

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

No. This is a test-only PR.

### How was this patch tested?

Run the above tests on M1.

Closes apache#34548 from dongjoon-hyun/SPARK-37282.

Authored-by: Dongjoon Hyun <dhyun@apple.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit a116045)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
sunchao pushed a commit to sunchao/spark that referenced this pull request Dec 8, 2021
…ndedLevelDBTest

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

This PR is a follow-up of apache#34548. This is missed due to `-Pyarn` profile.

### Why are the changes needed?

This is required to pass `yarn` module on Apple Silicon.

**BEFORE**
```
$ build/sbt "yarn/test"
...
[info] YarnShuffleServiceSuite:
[info] org.apache.spark.network.yarn.YarnShuffleServiceSuite *** ABORTED *** (20 milliseconds)
[info]   java.lang.UnsatisfiedLinkError: Could not load library. Reasons: [no leveldbjni64-1.8
...
```

**AFTER**
```
$ build/sbt "yarn/test" -Pyarn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedLevelDBTest
...
[info] Run completed in 4 minutes, 57 seconds.
[info] Total number of tests run: 135
[info] Suites: completed 18, aborted 0
[info] Tests: succeeded 135, failed 0, canceled 1, ignored 0, pending 0
[info] All tests passed.
[success] Total time: 319 s (05:19), completed Nov 12, 2021, 4:53:14 PM
```

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

No.

### How was this patch tested?

A manual test on Apple Silicon.
```
$ build/sbt "yarn/test" -Pyarn -Dtest.exclude.tags=org.apache.spark.tags.ExtendedLevelDBTest
```

Closes apache#34576 from dongjoon-hyun/SPARK-37282-2.

Authored-by: Dongjoon Hyun <dongjoon@apache.org>
Signed-off-by: Kousuke Saruta <sarutak@oss.nttdata.com>
(cherry picked from commit 8b45a08)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
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