-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Return index name and empty map for /{index}/_alias with no aliases #25114
Conversation
I skimmed the code, the solution looks good to me. However I think we need a REST test here, having a REST test already would have prevented the temporary (unreleased) breakage so I think we should at least add one now? I'm happy to give a full review when that's added. |
Thanks for taking a look @jasontedor, I pushed a commit that adds a REST test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I left one more comment, you can address and fire at will.
@@ -243,7 +243,7 @@ public boolean equalsAliases(MetaData other) { | |||
* | |||
* @param aliases The names of the index aliases to find | |||
* @param concreteIndices The concrete indexes the index aliases must point to order to be returned. | |||
* @return the found index aliases grouped by index | |||
* @return a map of index to a list of alias metadata, the list will be empty if no aliases are present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify this comment? Something like the list corresponding to a concrete index will be empty if no aliases are present for that index
?
Previously in elastic#24723 we changed the `_alias` API to not go through the `RestGetIndicesAction` endpoint, instead creating a `RestGetAliasesAction` that did the same thing. This changes the formatting so that it matches the old formatting of the endpoint, before: ``` GET /test-1/_alias { } ``` And after this change: ``` GET /test-1/_alias { "test-1": { "aliases": {} } } ``` This is related to elastic#25090
7714e97
to
5b2ab96
Compare
Thanks @jasontedor! |
These additional tests were previously reverted in 6.x due to failing bwc tests. They should be skipped against 5.6 nodes, bwc tests fail when the node hit by the request is on 5.6. Relates to elastic#29513 Relates to elastic#25114 Closes elastic#30806
Some recent failures on the mixed cluster tests were caused by elastic#31308. Instead of executing get index API when calling GET /_alias we now go through the get alias API. The behaviour of such API is slightly different on 5.6 compared to 6.x and master as to whether indices that have no aliases are returned or not. In fact elastic#25114 was not backported to 5.6. When the 5.6 node is the elected master, if the get alias API goes through such node or another 5.x node, the get index API will be used internally and all tests are fine. If some 6.x node is hit though by the client request, we will go through the get alias API, but we will do it through the elected master which will not return indices without aliases (at transport, see MetaData#findAliases on 5.6). That means that in a mixed cluster this API will return a different result depending on which node is the elected master and which one is hit by the request.
Some recent failures on the mixed cluster tests were caused by #31308. Instead of executing get index API when calling GET /_alias we now go through the get alias API. The behaviour of such API is slightly different on 5.6 compared to 6.x and master as to whether indices that have no aliases are returned or not. In fact #25114 was not backported to 5.6. When the 5.6 node is the elected master, if the get alias API goes through such node or another 5.x node, the get index API will be used internally and all tests are fine. If some 6.x node is hit though by the client request, we will go through the get alias API, but we will do it through the elected master which will not return indices without aliases (at transport, see MetaData#findAliases on 5.6). That means that in a mixed cluster this API will return a different result depending on which node is the elected master and which one is hit by the request.
Previously in #24723 we changed the
_alias
API to not go through theRestGetIndicesAction
endpoint, instead creating aRestGetAliasesAction
thatdid the same thing.
This changes the formatting so that it matches the old formatting of the
endpoint, before:
And after this change:
This is related to #25090