Skip to content

DynamoDB: Disable Scalable Vector Extensions in JRE #12112

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
Merged
merged 2 commits into from
Jan 9, 2025
Merged

DynamoDB: Disable Scalable Vector Extensions in JRE #12112

merged 2 commits into from
Jan 9, 2025

Conversation

viren-nadkarni
Copy link
Member

@viren-nadkarni viren-nadkarni commented Jan 8, 2025 •
edited
Loading

Background

There have been several reports of JRE crashing with SIGILL on Apple Silicon M4. This crash is specific to JRE 17+ when running in a Docker container (i.e. when prctl is not available) on aarch64.

In LocalStack, the bug affects DynamoDB Local.

Changes

This PR adds the suggested workaround of disabling Scalable Vector Extensions (SVE) in JRE.

SVE disable option is passed to the JVM only when supported, otherwise it complains about unsupported VM option and exits.

Tests

Due to lack of an M4 machine, it is not tested whether this resolves the issue.

Related

See https://bugs.openjdk.org/browse/JDK-8345296

Might close - #12054

@viren-nadkarni viren-nadkarni self-assigned this Jan 8, 2025
@viren-nadkarni viren-nadkarni added the semver: patch Non-breaking changes which can be included in patch releases label Jan 8, 2025
@viren-nadkarni viren-nadkarni marked this pull request as ready for review January 8, 2025 12:55
@viren-nadkarni viren-nadkarni marked this pull request as draft January 8, 2025 13:35
viren-nadkarni
viren-nadkarni commented Jan 9, 2025
View reviewed changes
localstack-core/localstack/services/dynamodb/server.py

# Workaround for JVM SIGILL crash on Apple Silicon M4
# See https://bugs.openjdk.org/browse/JDK-8345296
# To be removed after Java is bumped to 17.0.15+ and 21.0.7+
Copy link
Member Author

Choose a reason for hiding this comment

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

Eclipse Temurin haven't released the latest builds yet

https://github.com/adoptium/temurin17-binaries/releases

https://github.com/adoptium/temurin21-binaries/releases

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2025

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   27m 44s ⏱️ - 1h 24m 15s
1 073 tests  - 2 870  1 014 ✅  - 2 620  59 💤  - 250  0 ❌ ±0 
1 075 runs   - 2 870  1 014 ✅  - 2 620  61 💤  - 250  0 ❌ ±0 

Results for commit 861fe8b. ± Comparison against base commit 0191993.

This pull request removes 2870 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

@viren-nadkarni viren-nadkarni marked this pull request as ready for review January 9, 2025 08:49
alexrashed
alexrashed approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @viren-nadkarni for digging into this! I think this could be a very good lead to fix the issue, even though we unfortunately lack access to the hardware to actually test it on Apple M4.
Definitely worth a try, and we can ask users in #12054 to verify the blind fix once it's shipped in the latest image.

@viren-nadkarni viren-nadkarni merged commit fad716d into master Jan 9, 2025
37 checks passed
@viren-nadkarni viren-nadkarni deleted the dynamodb-aarch64 branch January 9, 2025 10:55
@tiurin tiurin mentioned this pull request Feb 4, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexrashed alexrashed alexrashed approved these changes

@giograno giograno Awaiting requested review from giograno

Assignees

@viren-nadkarni viren-nadkarni

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viren-nadkarni @alexrashed