Fix illegal reflective access warning from protostuff #373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
424609b to
9891c64
Compare
When using prostuff 1.7.4 under java9+ we get an Illegal reflective access warning due to protostuff 1.7.4's incompatibility with java9+. The issue is solved in protostuff/protostuff@b6ad96f and will be available in 1.8.0. Since protostuff 1.8.0 is not released, this PR built 1.8.0 packages and made AD depend on the local packages directly. With the change, the illegal reflective access is gone. Testing done: 1. Tested both single-stream and HCAD real-time detectors. 2. Tested backward compatibility: start a 1.2.0 cluster with protostuff 1.7.4, start detectors and wait for model checkpoints to be generated. Reinstall AD plugin with locally packaged protostuff 1.8.0. Verified tha old model can come up and detectors can produce results. Signed-off-by: Kaituo Li <kaituo@amazon.com>
ylwu-amzn
reviewed
Feb 8, 2022
| @@ -582,8 +582,10 @@ dependencies { | |||
| compile files('lib/randomcutforest-core-2.0.1.jar') | |||
|
|
|||
| // used for serializing/deserializing rcf models. | |||
| compile group: 'io.protostuff', name: 'protostuff-core', version: '1.7.4' | |||
| compile group: 'io.protostuff', name: 'protostuff-runtime', version: '1.7.4' | |||
| compile files('lib/protostuff-api-1.8.0-SNAPSHOT.jar') | |||
There was a problem hiding this comment.
Do you know when protostuff team plan to publish 1.8.0 to maven? We can move to public maven once it's ready.
There was a problem hiding this comment.
I don't. asked in their github repo, noboy replied.
There was a problem hiding this comment.
I couldn't find a link for the ask, opened protostuff/protostuff#327.
ylwu-amzn
approved these changes
Feb 8, 2022
amitgalitz
approved these changes
Feb 8, 2022
This was
linked to
issues
Feb 9, 2022
Closed
|
Just realized this package was compiled under JRE 11. Meaning code can no longer be run after compiling AD with anything below (JRE 8). Is this an issue? |
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
Mar 2, 2022
When using prostuff 1.7.4 under java9+ we get an Illegal reflective access warning due to protostuff 1.7.4's incompatibility with java9+. The issue is solved in protostuff/protostuff@b6ad96f and will be available in 1.8.0. Since protostuff 1.8.0 is not released, this PR built 1.8.0 packages and made AD depend on the local packages directly. With the change, the illegal reflective access is gone. Testing done: 1. Tested both single-stream and HCAD real-time detectors. 2. Tested backward compatibility: start a 1.2.0 cluster with protostuff 1.7.4, start detectors and wait for model checkpoints to be generated. Reinstall AD plugin with locally packaged protostuff 1.8.0. Verified tha old model can come up and detectors can produce results. Signed-off-by: Kaituo Li <kaituo@amazon.com> (cherry picked from commit 9315233)
ohltyler
pushed a commit
that referenced
this pull request
Mar 2, 2022
When using prostuff 1.7.4 under java9+ we get an Illegal reflective access warning due to protostuff 1.7.4's incompatibility with java9+. The issue is solved in protostuff/protostuff@b6ad96f and will be available in 1.8.0. Since protostuff 1.8.0 is not released, this PR built 1.8.0 packages and made AD depend on the local packages directly. With the change, the illegal reflective access is gone. Testing done: 1. Tested both single-stream and HCAD real-time detectors. 2. Tested backward compatibility: start a 1.2.0 cluster with protostuff 1.7.4, start detectors and wait for model checkpoints to be generated. Reinstall AD plugin with locally packaged protostuff 1.8.0. Verified tha old model can come up and detectors can produce results. Signed-off-by: Kaituo Li <kaituo@amazon.com> (cherry picked from commit 9315233)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Kaituo Li kaituo@amazon.com
Description
When using prostuff 1.7.4 under java9+ we get an Illegal reflective access warning due to protostuff 1.7.4's incompatibility with java9+. The issue is solved in protostuff/protostuff@b6ad96f and will be available in 1.8.0. Since protostuff 1.8.0 is not released, this PR built 1.8.0 packages and made AD depend on the local packages directly. With the change, the illegal reflective access is gone.
Testing done:
Issues Resolved
#263
#319
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.