Order of atomic alias operations seems inconsistent

[mayya-sharipova]

Executing an atomic alias operations on the same index and same alias returns inconsistent results:

# 1) Put an index
PUT some-concrete-index

# 2) no alias exists yet
GET some-concrete-index/_alias

# 3) add an alias, then remove it
POST /_aliases
{
  "actions": [
    {
      "add": {
        "index": "some-concrete-index",
        "alias": "oci-cmdb_service_members"
      }
    },
    {
      "remove": {
        "index": "some-concrete-index",
        "alias": "oci-cmdb_service_members"
      }
    }
  ]
}

# 4) Order shouldn't matter, and it does not seem to, as the alias now exists
GET some-concrete-index/_alias

#5) Execute the same operation again:
POST /_aliases
{
  "actions": [
    {
      "add": {
        "index": "some-concrete-index",
        "alias": "oci-cmdb_service_members"
      }
    },
    {
      "remove": {
        "index": "some-concrete-index",
        "alias": "oci-cmdb_service_members"
      }
    }
  ]
}

#6) The alias is removed now
GET some-concrete-index/_alias

To sum up:
The same request will produce different result:

1. If the alias doesn’t exist before the request, it will be created
2. If the alias doest exists, it will be removed

[bleskes]

I did some digging and this has to do with the fact that we treat aliases in the remove command as wild card expressions and resolve them in advance against the cluster state before we run the command. This means that if the alias was not there when we started, the remove command will never be executed when the cluster state update. I think this is trappy as there is no correlation between the cluster state that this pre filtering is run on and the cluster state which actually serves as base for the commands execution. On top of it it doesn't account for relationship between commands. I think we should not prefilter and resolve wild cards when we execute the commands on the cluster state thread.

[talevy]

I added the [discuss] label because I would like to discuss this in the context of eagerness to validate. This issue comes up with regards to #28231, and #30195. This also influences whether certain data-structures that are useful for early validation have value in being created as metadata is being changed and built by alias actions, rather than being re-created and validated all at once after all actions (whether legal or not, depending on the order) when the final MetaData is being built and validated (re: #29575).

to highlight some examples that all behave differently:

PUT foo

# add alias, then remove alias
# 1st call: alias created. 2nd call: alias removed
POST _aliases
{
  "actions": [
    { "add": { "index": "foo", "alias": "logs" } },
    { "remove": {"index": "foo", "alias": "logs" } }
  ]
}

# remove alias, then add alias
# after all calls: alias created
POST _aliases
{
  "actions": [
    { "remove": {"index": "foo", "alias": "logs" } },
    { "add": { "index": "foo", "alias": "logs" } }
  ]
}

# remove_index, then add alias
# after all calls: exception thrown when trying to add alias, index remains.
POST _aliases
{
  "actions": [
    { "remove_index": {"index": "foo"} },
    { "add": { "index": "foo", "alias": "logs" } }
  ]
}

# add alias, twice, updated
# after all calls: last alias metadata definition wins
POST _aliases
{
  "actions": [
    { "add": { "index": "foo","alias": "logs", "filter": { "exists": { "field": "FIELD_NAME" } }}},
    { "add": { "index": "foo","alias": "logs" }}
  ]
}

[tomcallahan]

@bleskes what would you like to do on this issue?

[bleskes]

@tomcallahan we discussed it in the core infra sync a while ago and agree that the actions should be applied one by one, modifying things as they go, based on the output of the previous command. If I recall correctly, the conversation went with validating correctness on every step. That said, is write index validation ended being done once in the end, so we might need to revisit it.

Bottom line - it's a valid issue and someone needs to pick it up. I expect more work here as the alias code needs some cleanup IMO. I believe that is also what made @mayya-sharipova fight an up hill battle with her PR.

[mayya-sharipova]

@bleskes To confirm, you were saying that we need to take another approach for this. So, I am going to close my previous PR on this, and somebody else can work on it.

[bleskes]

@mayya-sharipova I think your PR is an improvement but indeed, this should be picked up in a more structural way. I'm OK with you just closing it, if you're not going to spend more time in that area (and otherwise we need to finish that discussion we started)

[ddreonomy]

Does that mean that renaming an alias is not working properly?

https://www.elastic.co/guide/en/elasticsearch/reference/6.2/indices-aliases.html

POST /_aliases
{
    "actions" : [
        { "remove" : { "index" : "test1", "alias" : "alias1" } },
        { "add" : { "index" : "test2", "alias" : "alias1" } }
    ]
}

[mayya-sharipova]

@ddreonomy The renaming of alias should work well. If you experience any problem, please create a separate issue.
This PR is about adding/removing alias for the same index.