Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2759

rbhavna · 2024-07-26T17:34:39Z

Description

Adds the _doc key wrapper around index mapping as required by CreateIndexRequest javadoc.

Related Issues

See also opensearch-project/OpenSearch#14984

Testing Details

When the cluster has v1 templates with patterns matching the index, without the _doc wrapper, dynamic mappings are getting enabled for the index. From below, created_time is being created as long (dynamically picked) instead of timestamp type which is defined in the mapping definition.

With changes in this PR by adding _doc wrapper, even if cluster has v1 templates, indices are getting created with expected mappings.

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

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]>

ylwu-amzn · 2024-07-26T18:45:20Z

Have you checked any difference by wrapping index mapping with _doc or not ?

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

dbwiddis · 2024-07-26T18:48:16Z

@rbhavna please unlink the opensearch bug as there are other impacted plugins that need to track it

rbhavna · 2024-07-26T18:58:25Z

Have you checked any difference by wrapping index mapping with _doc or not ?

Yes when v1 template is added already, without _doc wrapper I see dynamic mappings are created. Will add details to PR description

dbwiddis · 2024-07-26T19:27:07Z

Have you checked any difference by wrapping index mapping with _doc or not ?

@ylwu-amzn I've tested all combinations (with and without _doc) and with/without v1 and v2 templates. The only failure occurs with the v1 template and lack of _doc. Adding _doc does not cause any additional problems and is, in fact, added by the REST API.

@owaiskazi19 and I walked through all code paths with both deprecated v1 and current v2 mappings. In v2 case and with no v1 mapping, code automatically determines the _doc type at run time and thus these not impacted as they are processed in a list. The only impact is with v1 (deprecated) index templates which do a map "merge" and because the template populates the _doc field, it is now "present" and the auto-populating that occurs when there's no _doc doesn't happen and the user mapping is lost.

owaiskazi19 · 2024-07-26T19:33:49Z

@rbhavna just to be sure, can you also test with v2 templates both the scenario?
Also, looks like you attached the same screenshot in the description

dbwiddis · 2024-07-26T19:37:00Z

memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java

@@ -87,7 +87,7 @@ public void initConversationMetaIndexIfAbsent(ActionListener<Boolean> listener)
            log.debug("No conversational meta index found. Adding it");
            CreateIndexRequest request = Requests
                .createIndexRequest(META_INDEX_NAME)
-                .mapping(ConversationalIndexConstants.META_MAPPING)
+                .mapping("{\"_doc\":" + ConversationalIndexConstants.META_MAPPING + "}")


FYI after further research, another possible (and less controversial) fix is using a differnet method that includes the MediaType, which auto-adds the doc field internally. This format is also used elsewhere.

Suggested change

.mapping("{\"_doc\":" + ConversationalIndexConstants.META_MAPPING + "}")

.mapping(ConversationalIndexConstants.META_MAPPING, XContentType.JSON)

rbhavna · 2024-07-26T19:40:21Z

@rbhavna just to be sure, can you also test with v2 templates both the scenario? Also, looks like you attached the same screenshot in the description

Thanks @owaiskazi19 Updated the description. And will do more testing with v2 templates

rbhavna · 2024-07-26T19:46:39Z

Have you checked any difference by wrapping index mapping with _doc or not ?

@ylwu-amzn I've tested all combinations (with and without _doc) and with/without v1 and v2 templates. The only failure occurs with the v1 template and lack of _doc. Adding _doc does not cause any additional problems and is, in fact, added by the REST API.

@owaiskazi19 and I walked through all code paths with both deprecated v1 and current v2 mappings. In v2 case and with no v1 mapping, code automatically determines the _doc type at run time and thus these not impacted as they are processed in a list. The only impact is with v1 (deprecated) index templates which do a map "merge" and because the template populates the _doc field, it is now "present" and the auto-populating that occurs when there's no _doc doesn't happen and the user mapping is lost.

I am actually curious if we can have similar fix for update index as well. This fix will take care of any newly created domains from 2.15. For domains upgrading from previous versions with incorrect mappings to 2.15, do you think adding this _doc wrapper helps?

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

…to fix v1 templates issue (#2759) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5)

…to fix v1 templates issue (#2759) (#2766) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]>

…to fix v1 templates issue (#2759) (#2763) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]>

…to fix v1 templates issue (#2759) (#2764) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]>

…to fix v1 templates issue (#2759) (#2765) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]>

…to fix v1 templates issue (#2759) (#2767) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]>

…to fix v1 templates issue (#2759) (#2766) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 389dd95)

…to fix v1 templates issue (#2759) (#2766) (#3049) * Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Signed-off-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 1c43be5) Co-authored-by: Bhavana Ramaram <[email protected]> (cherry picked from commit 389dd95) Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com>

Wrap CreateIndexRequest mappings in _doc key in ml-commons

a763379

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, samuel-oci and xinyual as code owners July 26, 2024 17:34

rbhavna temporarily deployed to ml-commons-cicd-env July 26, 2024 17:34 — with GitHub Actions Inactive

Zhangxunmt previously approved these changes Jul 26, 2024

View reviewed changes

rbhavna temporarily deployed to ml-commons-cicd-env July 26, 2024 18:40 — with GitHub Actions Inactive

add commit to release notes

cea9faf

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

rbhavna dismissed Zhangxunmt’s stale review via cea9faf July 26, 2024 18:47

rbhavna temporarily deployed to ml-commons-cicd-env July 26, 2024 18:47 — with GitHub Actions Inactive

rbhavna had a problem deploying to ml-commons-cicd-env July 26, 2024 18:47 — with GitHub Actions Failure

dbwiddis reviewed Jul 26, 2024

View reviewed changes

use XContentType to add the _doc wrapper

25572e0

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

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

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

rbhavna had a problem deploying to ml-commons-cicd-env July 26, 2024 22:14 — with GitHub Actions Failure

rbhavna changed the title ~~Wrap CreateIndexRequest mappings in _doc key in ml-commons~~ Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue Jul 26, 2024

ylwu-amzn approved these changes Jul 26, 2024

View reviewed changes

b4sjoo approved these changes Jul 26, 2024

View reviewed changes

rbhavna added backport 2.x backport 2.13 backport 2.14 backport 2.15 backport 2.16 labels Jul 26, 2024

ylwu-amzn approved these changes Jul 26, 2024

View reviewed changes

dhrubo-os approved these changes Jul 26, 2024

View reviewed changes

rbhavna merged commit 1c43be5 into opensearch-project:main Jul 26, 2024
9 of 10 checks passed

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

[Backport 2.x] Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2763

Merged

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

[Backport 2.13] Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2764

Merged

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

[Backport 2.14] Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2765

Merged

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

[Backport 2.15] Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2766

Merged

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

[Backport 2.16] Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2767

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2759

Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2759

rbhavna commented Jul 26, 2024 •

edited

Loading

ylwu-amzn commented Jul 26, 2024

dbwiddis commented Jul 26, 2024

rbhavna commented Jul 26, 2024

dbwiddis commented Jul 26, 2024 •

edited

Loading

owaiskazi19 commented Jul 26, 2024 •

edited

Loading

dbwiddis Jul 26, 2024 •

edited

Loading

rbhavna Jul 26, 2024

rbhavna commented Jul 26, 2024

rbhavna commented Jul 26, 2024

	.mapping("{\"_doc\":" + ConversationalIndexConstants.META_MAPPING + "}")
	.mapping(ConversationalIndexConstants.META_MAPPING, XContentType.JSON)

Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2759

Add XContentType to wrap the CreateIndexRequest mappings in _doc key to fix v1 templates issue #2759

Conversation

rbhavna commented Jul 26, 2024 • edited Loading

Description

Related Issues

Testing Details

Check List

ylwu-amzn commented Jul 26, 2024

dbwiddis commented Jul 26, 2024

rbhavna commented Jul 26, 2024

dbwiddis commented Jul 26, 2024 • edited Loading

owaiskazi19 commented Jul 26, 2024 • edited Loading

dbwiddis Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

rbhavna Jul 26, 2024

Choose a reason for hiding this comment

rbhavna commented Jul 26, 2024

rbhavna commented Jul 26, 2024

rbhavna commented Jul 26, 2024 •

edited

Loading

dbwiddis commented Jul 26, 2024 •

edited

Loading

owaiskazi19 commented Jul 26, 2024 •

edited

Loading

dbwiddis Jul 26, 2024 •

edited

Loading