Disallow duplicate template names in `dynamic_template`

cbuescher commented 4 years ago

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 (https://github.com/elastic/apm-server/issues/2693).

This creates problems and bugs when these duplicate templates are merged e.g. during inheritance like described in https://github.com/elastic/apm-server/issues/2693#issuecomment-530366857.

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?

elasticmachine commented 4 years ago

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

jtibshirani commented 4 years ago

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 commented 4 years ago

APM Server currently makes use of multiple dynamic templates with the same key. Linking https://github.com/elastic/beats/issues/17203#issuecomment-603436023 here.

romain-chanu commented 2 years ago

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.

elasticsearchmachine commented 2 months ago

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

elastic / elasticsearch