update config index mappings to use correct field types #2710

rbhavna · 2024-07-23T09:11:31Z

Description

update config index mappings to use correct field types.

In 2.13, config index mapping was updated with three new fields configuration, type, and last_update_timebut schema version was not bumped. Due to which, the domains which were upgraded to 2.13 from previous versions did not go through update index mapping as they did not detect a schema version bump.

During indexing, these fields got created with wrong types not as specified in the mapping. Now when such domains are going to upgrade to 2.15, with schema_version bump, the index mapping tries to get updated but will throw exception since the field types for the three fields specified above are different. Hence the index initialization fails and indexing into the config index wouldn't be possible.

Hence, the PR tries to add complete new fields(ml_configuration, config_type, last_updated_time while removing the old ones. So whenever mapping is getting updated, since there is no type-conflict for old fields, they get retained in the index as is while new fields get added. It is therefore suggested to start using these new fields from 2.15 while indexing into config index.

Issues Resolved

#2630

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
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.

Signed-off-by: Bhavana Ramaram <[email protected]>

jngz-es · 2024-07-23T18:16:53Z

Hi @rbhavna , I don't quite get why we change the field names for the type and configuration. Could you explain a little bit?

ylwu-amzn · 2024-07-23T20:54:42Z

common/src/main/java/org/opensearch/ml/common/MLConfig.java

                case CONFIGURATION_FIELD:
                    configuration = Configuration.parse(parser);
                    break;
+                case ML_CONFIGURATION_FIELD:


Another field added in 2.13, LAST_UPDATE_TIME_FIELD, let's use similar way to fix?

Signed-off-by: Bhavana Ramaram <[email protected]>

rbhavna · 2024-07-23T21:12:55Z

Hi @rbhavna , I don't quite get why we change the field names for the type and configuration. Could you explain a little bit?

added details to PR description

Zhangxunmt · 2024-07-23T23:04:20Z

common/src/main/java/org/opensearch/ml/common/MLConfig.java

    public static final String CREATE_TIME_FIELD = "create_time";
    public static final String LAST_UPDATE_TIME_FIELD = "last_update_time";

+    public static final String LAST_UPDATED_TIME_FIELD = "last_updated_time";


Is it possible to reuse the old Field names for CX experience BWC, rather than defining new ones?

Using old names and trying to update their types is failing. OpenSearch does not allow conversion between certain types and flat_object is one of them

* update config index mappings to use with correct field types Signed-off-by: Bhavana Ramaram <[email protected]> * enable dynamic mapping for config index Signed-off-by: Bhavana Ramaram <[email protected]> * change last_update_time field Signed-off-by: Bhavana Ramaram <[email protected]> --------- Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1926699)

* update config index mappings to use with correct field types Signed-off-by: Bhavana Ramaram <[email protected]> Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1926699) Co-authored-by: Bhavana Ramaram <[email protected]>

…roject#2710) * update config index mappings to use with correct field types Signed-off-by: Bhavana Ramaram <[email protected]> * enable dynamic mapping for config index Signed-off-by: Bhavana Ramaram <[email protected]> * change last_update_time field Signed-off-by: Bhavana Ramaram <[email protected]> --------- Signed-off-by: Bhavana Ramaram <[email protected]>

ylwu-amzn · 2024-09-03T20:43:34Z

common/src/main/java/org/opensearch/ml/common/MLConfig.java

@@ -78,10 +85,10 @@ public void writeTo(StreamOutput out) throws IOException {
    public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException {
        XContentBuilder builder = xContentBuilder.startObject();
        if (type != null) {
-            builder.field(TYPE_FIELD, type);
+            builder.field(CONFIG_TYPE_FIELD, type);


Before this PR

"type" : "os_query_assist_ppl_agent", "configuration" : { "agent_id" : "qdFHuZEBJv7K-ogaOTOB" }

Now we have

"config_type" : "os_query_assist_ppl_agent", "ml_configuration" : { "agent_id" : "qdFHuZEBJv7K-ogaOTOB" }

This is a breaking change.

How about changing to

if (type != null) { builder.field(TYPE_FIELD, type); } if (configType != null) { builder.field(CONFIG_TYPE_FIELD, type); }

Same for line 91, that's also breaking

ylwu-amzn · 2024-09-03T20:59:12Z

common/src/main/java/org/opensearch/ml/common/MLConfig.java

                default:
                    parser.skipChildren();
                    break;
            }
        }
        return MLConfig.builder()
-                .type(type)
-                .configuration(configuration)
+                .type(configType == null ? type : configType)


Parse new field value to new field

.type(type) .configType(configType)

…roject#2710) (opensearch-project#2718) * update config index mappings to use with correct field types Signed-off-by: Bhavana Ramaram <[email protected]> Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1926699) Co-authored-by: Bhavana Ramaram <[email protected]>

… (#3053) * update config index mappings to use with correct field types (cherry picked from commit 1926699) Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Bhavana Ramaram <[email protected]>

update config index mappings to use with correct field types

391bcf9

Signed-off-by: Bhavana Ramaram <[email protected]>

rbhavna requested review from b4sjoo, dhrubo-os, jngz-es, model-collapse, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27, sam-herman and xinyual as code owners July 23, 2024 09:11

rbhavna temporarily deployed to ml-commons-cicd-env July 23, 2024 09:11 — with GitHub Actions Inactive

rbhavna temporarily deployed to ml-commons-cicd-env July 23, 2024 10:11 — with GitHub Actions Inactive

ylwu-amzn reviewed Jul 23, 2024

View reviewed changes

enable dynamic mapping for config index

5513c27

Signed-off-by: Bhavana Ramaram <[email protected]>

rbhavna temporarily deployed to ml-commons-cicd-env July 23, 2024 20:57 — with GitHub Actions Inactive

rbhavna temporarily deployed to ml-commons-cicd-env July 23, 2024 20:58 — with GitHub Actions Inactive

change last_update_time field

64dbbd2

Signed-off-by: Bhavana Ramaram <[email protected]>

rbhavna temporarily deployed to ml-commons-cicd-env July 23, 2024 21:04 — with GitHub Actions Inactive

rbhavna temporarily deployed to ml-commons-cicd-env July 23, 2024 22:04 — with GitHub Actions Inactive

Zhangxunmt reviewed Jul 23, 2024

View reviewed changes

Zhangxunmt approved these changes Jul 23, 2024

View reviewed changes

ylwu-amzn approved these changes Jul 23, 2024

View reviewed changes

ylwu-amzn merged commit 1926699 into opensearch-project:main Jul 24, 2024
6 checks passed

ylwu-amzn added the backport 2.x label Jul 24, 2024

ylwu-amzn added backport 2.16 backport 2.15 labels Jul 24, 2024

opensearch-trigger-bot bot mentioned this pull request Jul 24, 2024

[Backport 2.x] update config index mappings to use correct field types #2716

Merged

opensearch-trigger-bot bot mentioned this pull request Jul 24, 2024

[Backport 2.16] update config index mappings to use correct field types #2717

Merged

opensearch-trigger-bot bot mentioned this pull request Jul 24, 2024

[Backport 2.15] update config index mappings to use correct field types #2718

Merged

qianheng-aws mentioned this pull request Jul 24, 2024

switch to ml_configuration in agent response as backend changed opensearch-project/dashboards-assistant#239

Merged

4 tasks

b4sjoo added the v2.16.0 Issues targeting release v2.16.0 label Jul 26, 2024

joshuali925 mentioned this pull request Sep 3, 2024

[query assist] update api handler to accommodate new ml-commons config response opensearch-project/dashboards-observability#2111

Merged

6 tasks

ylwu-amzn reviewed Sep 3, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update config index mappings to use correct field types #2710

update config index mappings to use correct field types #2710

rbhavna commented Jul 23, 2024 •

edited

Loading

jngz-es commented Jul 23, 2024

ylwu-amzn Jul 23, 2024

rbhavna commented Jul 23, 2024

Zhangxunmt Jul 23, 2024

rbhavna Jul 24, 2024

ylwu-amzn Sep 3, 2024 •

edited

Loading

ylwu-amzn Sep 3, 2024

ylwu-amzn Sep 3, 2024 •

edited

Loading

update config index mappings to use correct field types #2710

update config index mappings to use correct field types #2710

Conversation

rbhavna commented Jul 23, 2024 • edited Loading

Description

Issues Resolved

Check List

jngz-es commented Jul 23, 2024

ylwu-amzn Jul 23, 2024

Choose a reason for hiding this comment

rbhavna commented Jul 23, 2024

Zhangxunmt Jul 23, 2024

Choose a reason for hiding this comment

rbhavna Jul 24, 2024

Choose a reason for hiding this comment

ylwu-amzn Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

ylwu-amzn Sep 3, 2024

Choose a reason for hiding this comment

ylwu-amzn Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

rbhavna commented Jul 23, 2024 •

edited

Loading

ylwu-amzn Sep 3, 2024 •

edited

Loading

ylwu-amzn Sep 3, 2024 •

edited

Loading