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

Revise get aliases response #30536

Closed
javanna opened this issue May 11, 2018 · 7 comments
Closed

Revise get aliases response #30536

javanna opened this issue May 11, 2018 · 7 comments
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team

Comments

@javanna
Copy link
Member

javanna commented May 11, 2018

The get aliases API response is somewhat unique. As some other API, it mixes errors with valid aliases returned, which makes it quite hard to reason about. I would like us to review whether we want to make any change.

The usual response is something like the following:

curl localhost:9200/index*/_alias?pretty
{
  "index1" : {
    "aliases" : {
      "alias1" : { }
    }
  },
  "index2" : {
    "aliases" : { }
  },
  "index3" : {
    "aliases" : {
      "alias3" : { }
    }
  }
}

In case a requested index doesn't exist, an error gets returned (status 404):

curl localhost:9200/does_not_exist/_alias?pretty
{
  "error" : {
    "root_cause" : [
      {
        "type" : "index_not_found_exception",
        "reason" : "no such index",
        "resource.type" : "index_or_alias",
        "resource.id" : "does_not_exist",
        "index_uuid" : "_na_",
        "index" : "does_not_exist"
      }
    ],
    "type" : "index_not_found_exception",
    "reason" : "no such index",
    "resource.type" : "index_or_alias",
    "resource.id" : "does_not_exist",
    "index_uuid" : "_na_",
    "index" : "does_not_exist"
  },
  "status" : 404
}

When some requested aliases are not found though, the response becomes the following if no aliases are found:

curl 'localhost:9200/_alias/b*?pretty'
{
  "error" : "alias [b*] missing",
  "status" : 404
}

and becomes the following when some aliases are not found and some others are found:

curl 'localhost:9200/_alias/a*,b*?pretty'
{
  "error" : "alias [b*] missing",
  "status" : 404,
  "index1" : {
    "aliases" : {
      "alias1" : { }
    }
  },
  "index2" : {
    "aliases" : {
      "alias2" : { }
    }
  }
}

The error in the response above should probably be wrapped in another object, and the status returned is 404 although some aliases have been returned. Not sure whether that is intended. This was found as part of #28799 as this kind of responses is quite tricky to parse back when adding support for them to the high-level REST client. It is not clear whether we should throw an exception whenever 404 is returned, although in the last case some aliases were found and they would be lost if the client threw an exception. The other problem is that the structure of the "alias not found" error is not compliant to the usual errors that can be parsed back as an ElasticsearchException, which forces us to parse it back as a String. In fact on the server side it is not an exception, it is just how the response is rendered back at REST.

This seems like an example of API that can return "partial results", then the response should probably hold a failure object together with the aliases found?

@javanna javanna added discuss :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. labels May 11, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@jasontedor jasontedor added :Data Management/Indices APIs APIs to create and manage indices and templates and removed :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. labels May 11, 2018
@jasontedor
Copy link
Member

I agree, this does require some rethinking. This arose from #25043 which was a solution for #24644. To summarize:

  • our HEAD and GET responses were not aligned; HEAD should always match the output of GET only without the body (e.g., the status code and Content-Length header should be the same)
  • historically, HEAD alias was used to check for the existence of an alias and would 404 if an alias was missing
  • when we aligned HEAD and GET alias, this broke the use of HEAD alias as an existence check as GET alias did not behave in this way
  • to return the use of HEAD alias, we made GET alias behave similarly

This is how we got to where we are today. That said, this is not at all consistent with our other APIs and I agree we should move it to something like you describe with an error object and a realignment of the status code.

@javanna javanna changed the title Review get aliases response Revise get aliases response May 15, 2018
@javanna
Copy link
Member Author

javanna commented May 15, 2018

thanks @jasontedor for the history. One more thing I recently stumbled upon is that we may or may not return indices without matching aliases, depending on whether an alias was provided in the request or not (#27763). I realize there is even more history around this, but I find that a bit confusing, maybe it is a good time to discuss that too if we revise the get aliases response.

@pcsanwald pcsanwald removed the discuss label Jun 7, 2018
javanna pushed a commit that referenced this issue Jun 12, 2018
Given the weirdness of the response returned by the get alias API, we went for a client specific response, which allows us to hold the error message, exception and status returned as part of the response together with aliases. See #30536 .

Relates to #27205
javanna pushed a commit that referenced this issue Jun 13, 2018
Given the weirdness of the response returned by the get alias API, we went for a client specific response, which allows us to hold the error message, exception and status returned as part of the response together with aliases. See #30536 .

Relates to #27205
@albertzaharovits
Copy link
Contributor

albertzaharovits commented Oct 3, 2018

We are working on getting multi index expressions for the GetAlias API (/{index}/_alias/{name} and /_alias/{name}) #33518 #34144 #34230 . The expressions would contain wildcard exclusions (-...*) for alias names (the {name} parameter).

But, this is touching on a topic being discussed here. And it is stalling #34230 .

The problem at hand is that 404s are being returned for wildcards.
For example it is perfectly reasonable for the code to return 404 if a requested
wildcard was not expanded to any concrete alias:
GET _alias/foo* when there's no such alias, the API returns:

{
  "error" : "aliases [foo*] missing",
  "status" : 404
}

This is the present behavior (hard to interpret by the REST client, but nice).

It is important, for the following, to mention that this behavior is entirely
generated by the rest handler RestGetAliasesAction, the underlying transport
action is not erroring anything.

The present behavior, is troubling when we allow for (wildcard) exclusions.
Wildcard exclusions could remove aliases that have been expanded from
previous wildcards and the API would report the wildcard as missing if
it happens that all the expanded aliases have been subsequently excluded.
For example:

# setup
curl -X PUT "localhost:9200/index" -H "Content-Type: application/json" -d'
{
  "aliases": {
    "foo1": {},
    "foo2": {},
    "foobar1": {},
    "foobar2": {}
  }
}

curl -X GET "localhost:9200/_alias/foob*,-foo*?pretty"
{
  "error" : "aliases [-foo*,foob*] missing",
  "status" : 404
}

curl -X GET "localhost:9200/_alias/foob*,-foobar*?pretty"
{
  "error" : "aliases [-foobar*,foob*] missing",
  "status" : 404
}

Sure, the broken behavior can be triggered more simply because
the code does not handle exclusions at all:

curl -X GET "localhost:9200/_alias/foob*,-foobar1?pretty"
{
  "error" : "alias [-foobar1] missing",
  "status" : 404,
  "index" : {
    "aliases" : {
      "foobar2" : { }
    }
  }
}

The result is correct but the 404 is wrong.

This is something obviously broken, that does not require any debate,
and I've been trying to fix it in #34230.

The problem is that the fix requires breaking the current good behavior.

If a wildcard is followed by an exclusion wildcard there is no way to say if
the exclusion eliminated all the ones previously included or that the inclusion
wildcard was empty to begin with. For example:

curl -X GET "localhost:9200/_alias/foob*,-foobar*?pretty"
{
  "error" : "aliases [-foobar*,foob*] missing",
  "status" : 404
}
 curl -X GET "localhost:9200/_alias/foobar*,-foob*?pretty"
{
  "error" : "aliases [-foob*,foobar*] missing",
  "status" : 404
}

should both return an empty result, as they do, but without the missing messages,
because nothing is missing, foobar1 and foobar2 were included and then excluded.
However, if there were no foob* aliases to begin with, then indeed the
"aliases [foobar*] missing" message would be appropriate.
Trouble is that this logic, dealing with 404s and messages, does not have access
to the existing aliases, as it sits in the REST handler.

Bottom line, I am proposing to remove reporting aliases as missing altogether.
Only explicitly requested aliases would trigger a missing msg and a 404. This is
implemented in #34230 but we think it requires more thumbs up from the team
before moving forward.

@jasontedor your opinion would be especially valuable.

CC @javanna

@joegallo
Copy link
Contributor

I'm removing the team-discuss label from some older Team:Data Management issues -- we've had plenty of time to discuss them, but we haven't, so the label isn't serving its purpose. Feel free to delete this comment and/or re-add the team-discuss label.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@dakrone
Copy link
Member

dakrone commented May 8, 2024

I believe this is resolved somewhat by #106514, so I am closing this for now.

@dakrone dakrone closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates Team:Data Management Meta label for data/management team
Projects
None yet
Development

No branches or pull requests

9 participants