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

Disallow duplicate template names in dynamic_template #53326

Open
cbuescher opened this issue Mar 10, 2020 · 5 comments
Open

Disallow duplicate template names in dynamic_template #53326

cbuescher opened this issue Mar 10, 2020 · 5 comments
Labels
>breaking :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch

Comments

@cbuescher
Copy link
Member

Today, dynamic templates can contain multiple template names with the same name. This can be the result of either adding the same template multiple times in error (like the case described in #28988 or explicitly to group small variations of the same template like mentioned in this example of an APM index templates (elastic/apm-server#2693).

This creates problems and bugs when these duplicate templates are merged e.g. during inheritance like described in elastic/apm-server#2693 (comment).

Instead, all names should be a unique ID, and we should fail when there are duplicates.
To make this change in a backwards compatible way, we should start logging warnings about this problem starting in one of the next 7.x releases.

In order to make sure that existing dynamic templates (e.g. inside index templates) still work when upgrading to 8, and we are late in the 7.x series, we need to support creating indices from “broken” existing templates (like ones uploaded in 7.1) until 8.latest.

At the same time, from now on we can start warning users when they create index templates or indix mapping containing duplicates in the dynamic_templates section. In 8.x we should start rejecting new index templates and index mappings with duplicate names, (potentially accept but forcefully rewrite them to unique IDs if request is made with version 7 compatibility requested - to be discussed)

In 9 we should aim at rejecting broken templates and new indices created from them entirely.

Open questions:

  • If we allow new indices to be created from existing 7.x index templates that contain “broken” dynamic templates sections, we again have these indices in the 8.x cluster. We then might need to support them going forward to 9?
  • How can we remove / rewrite existing index templates containing “broken” dynamic templates sections going forward. If e.g. folks do rolling upgrades from 7 -> 8 -> 9 they need to be rewritten at some point?
@cbuescher cbuescher added the :Search Foundations/Mapping Index mappings, including merging and defining field types label Mar 10, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Mapping)

@jtibshirani
Copy link
Contributor

It's great we're moving this forward, there have been some long-standing issues in this area! Tagging @elastic/beats and @elastic/apm-server so they're aware of the above proposal to disallow having two dynamic_templates with the same name.

@simitt
Copy link
Contributor

simitt commented May 29, 2020

APM Server currently makes use of multiple dynamic templates with the same key. Linking elastic/beats#17203 (comment) here.

@romain-chanu
Copy link

romain-chanu commented Jul 2, 2022

This has created various bugs/problems observed in the field.

1. Observations

We have observed inconsistent behaviours in both versions 7.17.5 and 8.3.1 when regular indices or data streams are used.

1.1 Regular index with legacy index template

PUT _template/mytemplate
{
  "index_patterns": [
    "myindex-*"
  ],
  "mappings": {
    "dynamic_templates": [
      {
        "double_as_long": {
          "path_match": "myobject.abc.*",
          "match_mapping_type": "double",
          "mapping": {
            "type": "long"
          }
        }
      },
      {
        "double_as_long": {
          "path_match": "myobject.def.*",
          "match_mapping_type": "double",
          "mapping": {
            "type": "long"
          }
        }
      }
    ]
  }
}
PUT myindex-1
GET myindex-1/_mapping

Results:

{
  "myindex-1": {
    "mappings": {
      "dynamic_templates": [
        {
          "double_as_long": {
            "path_match": "myobject.abc.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        },
        {
          "double_as_long": {
            "path_match": "myobject.def.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        }
      ]
    }
  }
}

1.2 Regular index with composable index template

PUT _index_template/mytemplate
{
  "index_patterns": [
    "myindex-*"
  ],
  "template": {
    "mappings": {
      "dynamic_templates": [
        {
          "double_as_long": {
            "path_match": "myobject.abc.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        },
        {
          "double_as_long": {
            "path_match": "myobject.def.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        }
      ]
    }
  }
}
PUT myindex-1
GET myindex-1/_mapping

Results:

{
  "myindex-1": {
    "mappings": {
      "dynamic_templates": [
        {
          "double_as_long": {
            "path_match": "myobject.abc.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        },
        {
          "double_as_long": {
            "path_match": "myobject.def.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        }
      ]
    }
  }
}

1.3 Data stream with composable index template

DELETE _index_template/mytemplate

PUT _index_template/mytemplate
{
  "index_patterns": [
    "myindex-*"
  ],
  "template": {
    "mappings": {
      "dynamic_templates": [
        {
          "double_as_long": {
            "path_match": "myobject.abc.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        },
        {
          "double_as_long": {
            "path_match": "myobject.def.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        }
      ]
    }
  },
  "data_stream": { }
}
PUT _data_stream/myindex-1
GET .ds-myindex-1-2022.07.02-000001/_mapping

Results:

{
  ".ds-myindex-1-2022.07.02-000001": {
    "mappings": {
      "_data_stream_timestamp": {
        "enabled": true
      },
      "dynamic_templates": [
        {
          "double_as_long": {
            "path_match": "myobject.def.*",
            "match_mapping_type": "double",
            "mapping": {
              "type": "long"
            }
          }
        }
      ],
      "properties": {
        "@timestamp": {
          "type": "date"
        }
      }
    }
  }
}

2. Moving forward

  1. Implementation-wise, this has to be discussed as per the initial post.

  2. In the meantime, we should update the documentation (c.f Dynamic templates) to recommend using unique template names in the dynamic_templates array.

@javanna javanna added Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch and removed Team:Search Meta label for search team labels Jul 16, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-foundations (Team:Search Foundations)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch
Projects
None yet
Development

No branches or pull requests

8 participants