Skip to content
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.

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

[BUG] IllegalArgumentException in AD Settings registration #792

Closed
dbwiddis opened this issue May 30, 2023 · 5 comments
Closed

[BUG] IllegalArgumentException in AD Settings registration #792

dbwiddis opened this issue May 30, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@dbwiddis
Copy link
Member

What is the bug?

20:20:43.642 [opensearch[ad-extension][generic][T#4]] DEBUG org.opensearch.transport.TransportService - Action: internal:discovery/enviornmentsettings
20:20:43.647 [opensearch[ad-extension][generic][T#2]] INFO  org.opensearch.sdk.handlers.AcknowledgedResponseHandler - Extension Request failed
org.opensearch.transport.RemoteTransportException: [ip-10-0-134-13.ec2.internal][10.0.134.13:9300][internal:discovery/registercustomsettings]
Caused by: java.lang.IllegalArgumentException: non-node-scoped setting [opendistro.anomaly_detection.enabled] can not have property [Consistent]
        at org.opensearch.common.settings.Setting.checkPropertyRequiresNodeScope(Setting.java:219) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.Setting.<init>(Setting.java:206) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.Setting.<init>(Setting.java:250) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.Setting.<init>(Setting.java:231) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.Setting.<init>(Setting.java:285) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.Setting.boolSetting(Setting.java:2001) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.WriteableSetting.createSetting(WriteableSetting.java:169) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.common.settings.WriteableSetting.<init>(WriteableSetting.java:113) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
        at org.opensearch.extensions.settings.RegisterCustomSettingsRequest.<init>(RegisterCustomSettingsRequest.java:42) ~[opensearch-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]

How can one reproduce the bug?

Start up AD feature/extensions branch and init with connection to OpenSearch

What is the expected behavior?

No exceptions

@dbwiddis dbwiddis added bug Something isn't working untriaged labels May 30, 2023
@dbwiddis
Copy link
Member Author

dbwiddis commented Jun 1, 2023

This is due to the stale 3.0.0 latest build in use for AD testing, which does not have the change to the Setting.Property enum that was put into the middle of the enum order in opensearch-project/OpenSearch#7526.

Normally enums in Java are type safe and re-orderable, but in the case of settings sent over transport, the stream uses ordinal() so ordering matters.

This SDK bug will self-resolve when the 3.0.0 tarball build gets fixed, but I'm thinking that this change in enum values could definitely have some impact in versioning and backwards compatibility (possibly in 2.8.0 vs. 2.7.0?)

@saratvemulapalli and @cwperks do you think we should consider moving that new ExtensionScope enum value to the end of the list? Is this a potential issue for 2.8.0 release? Since we haven't "released" extensions yet and I don't think OpenSearch is sending setting properties over transport this is probably not an issue there, but it's definitely something we should keep an eye out for in code reviews.

@dbwiddis dbwiddis removed the untriaged label Jun 1, 2023
@dbwiddis dbwiddis self-assigned this Jun 1, 2023
@cwperks
Copy link
Member

cwperks commented Jun 1, 2023

@dbwiddis Is there a way we can synthetically reproduce this in a unit test to show a fix working by adding this to the end of the enum?

@dbwiddis
Copy link
Member Author

dbwiddis commented Jun 1, 2023

just implement writeable and do a writeTo ... that gives you a byte array with the enum ordinals

@dbwiddis
Copy link
Member Author

dbwiddis commented Jun 1, 2023

The current bug is that a NodeScope setting with the "old" enum value now gets deserialized as an ExtensionScope.

@dbwiddis
Copy link
Member Author

dbwiddis commented Jun 1, 2023

Fixed in opensearch-project/OpenSearch#7871

@dbwiddis dbwiddis closed this as completed Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants