Rename settings from OpenDistro and OpenSearch, with backwards compatibility, using fallback

[dblock]

UPDATE 5/19/21: the names for settings in this PR are opensearch.(e.g. opensearch.jobscheduler.retry_count), but have decided to make those names plugins.jobscheduler.retry_count, since. Please see https://github.com/opensearch-project/opensearch-plugins/blob/main/CONVENTIONS.md#settings for the latest.

---

Signed-off-by: dblock [email protected]

Description

1. Rename setting from opendistro to opensearch.
2. Use opendistro settings as fallback.

An alternative to #19, depends on opensearch-project/OpenSearch#665.

opensearch.yml Settings

Manually testing upgrade path from config/opensearch.yml settings:

Set opendistro.jobscheduler.retry_count: 6 in config./opensearch.yml.

tar vfxz opensearch-1.0.0-beta1-linux-x64.tar.gz 
cd opensearch-1.0.0-beta1
./bin/opensearch-plugin install opensearch-job-scheduler-1.0.0.0-beta1.zip 

Run ./bin/opensearch, it will show the value for this setting at 7.

Node Settings (and upgrade from OpenDistro)

Install OpenDistro

wget https://d3g5vo6xdbdb9a.cloudfront.net/tarball/opendistro-elasticsearch/opendistroforelasticsearch-1.13.2-linux-x64.tar.gz
tar vfxz opendistroforelasticsearch-1.13.2-linux-x64.tar.gz
cd opendistroforelasticsearch-1.13.2
./bin/elasticsearch

Set opendistro.jobscheduler.retry_count: 6.

curl -X PUT "localhost:9200/_cluster/settings?pretty" -H 'Content-Type: application/json' -d'{"persistent":{"opendistro.jobscheduler.retry_count":6}}'

Stop OpenDistro, install OpenSearch on top (or copy data). I built just the minimal engine + job-scheduler from source with these changes.

tar vfxz opensearch-1.0.0-beta1-linux-x64.tar.gz 
cd opensearch-1.0.0-beta1
./bin/opensearch-plugin install opensearch-job-scheduler-1.0.0.0-beta1.zip 
rm -rf data
cp -R ../opendistroforelasticsearch-1.13.2/data .

At start the default values are used before settings are read.

[2021-05-06T19:55:17,649][INFO ][c.a.o.j.s.JobSweeper     ] [ip-172-31-41-248] Background sweep search backoff retry count: 3

I thought this was a bug, but both old and new are just defaulted before they are read from the node configuration when it joins the cluster (with some debugging code).

[2021-05-06T19:55:17,651][INFO ][c.a.o.j.s.JobSweeper     ] [ip-172-31-41-248] JobSchedulerSettings.SWEEP_BACKOFF_RETRY_COUNT: 3
[2021-05-06T19:55:17,651][INFO ][c.a.o.j.s.JobSweeper     ] [ip-172-31-41-248] LegacyOpenDistroJobSchedulerSettings.SWEEP_BACKOFF_RETRY_COUNT: 3

As the node joins the cluster, it updates the value from the node configuration.

[2021-05-06T19:55:18,696][INFO ][o.o.c.s.ClusterSettings  ] [ip-172-31-41-248] updating [opensearch.jobscheduler.retry_count] from [3] to [6]

Check List

• New functionality includes testing.
  • All tests pass
• New functionality has been documented.
  • New functionality has javadoc added
• Passing build, needs changes in OpenSearch
• Commits are signed per the DCO using --signoff

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.

[Bukhtawar]

Thanks @dblock for the changes, Overall LGTM

1. Would it make sense to add ITs to this and see how would settings update via API behave with fallback.
2. While this is good was wondering if we simply had a way to say key.replaceFirst("^opendisto", "opensearch")

[Bukhtawar]

Lets use LegacyOpenDistroJobSchedulerSettings constants here maybe

[dblock]

I actually would rather not. If I were to make a mistake or a typo in the actual rename in the setting, or someone accidentally bulk replaces opendistro by openseaerch, the test will point to a behavior change or start failing. Which is what we really want. WDYT?

[dblock]

Thanks @dblock for the changes, Overall LGTM

1. Would it make sense to add ITs to this and see how would settings update via API behave with fallback.

Probably. I'll take a look at what's possible.

2. While this is good was wondering if we simply had a way to say key.replaceFirst("^opendisto", "opensearch")

The settings infrastructure supports upgrades, fallbacks, archiving settings, and a lot more. So some kind of s/opendistro/opensearch/g just felt too magical to me, and smelled like something that would break in very unexpected ways. It also wouldn't easily provide a visible deprecation path, such as existing well-understood built-in warnings or mechanisms designed and tested over many versions.

[saratvemulapalli]

Overall the changes look good the me!

Thanks for the change!

[saratvemulapalli]

From the changes, I see we are leveraging fallbackSetting to take care of legacy settings.
Ref: https://github.com/opensearch-project/OpenSearch/blob/0ba0e7cc26060f964fcbf6ee45bae53b3a9941d0/server/src/main/java/org/opensearch/common/settings/Setting.java#L1703:38

Moving to 1.x might not be needed.
But eventually we will have to build out of 1.x.

Let me know your thoughts on this, I propose:

1. cut a branch for 1.0.0.0-beta1 for all the plugins.
2. 1.0.0.0-beta1 will still build out of OpenSearch 1.0.0-beta1, this helps having stable builds.
3. main will build out of the latest feature branch on OpenSearch, in this case 1.x, this helps absorbing changes from dependencies sooner rather than later.

[dblock]

I think I'm confused what's what :)

Right now OpenSearch#main is 2.0 and OpenSearch#1.x is 1.0, right? Beta1 is old news.

This change depends on opensearch-project/OpenSearch#665, ie. opensearch-project/OpenSearch@b1b563d. Right now that's in main and 1.x.

So what should be here?

[saratvemulapalli]

Ah that makes sense. I didn't realize you've added changes to support fallback settings :).
Makes sense. Ignore my comments above.

[dblock]

@Bukhtawar I tried to write an integration test across multiple versions of ODFE/ES/OpenSearch, but that looks complicated. I opened opensearch-project/OpenSearch#670, if you know how to best do that pls add comments.