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

[REST API] «/<index_name>-*/_status» + «curator» ≈ massive data loss #9081

Closed
linux01d opened this issue Dec 29, 2014 · 7 comments

Comments

@linux01d
Copy link

commented Dec 29, 2014

curator 1.0.0 still uses «localhost:9200/<index_name>-_/status», which is deprecated in ES 1.2.0
Anyway, ES has a bug in /<index_name>-
/_status — it returns the list of ALL indices if wildcard doesn't match to any index and this may lead to data loss if curator takes a list of all existent indices and attempts to remove them.
In our installation we have lost 40T of data when last index named «intape-logstash-YYYY.MM.DD» has gone by the next time cron started job «curator -p intape-logstash- -C space -g1000».

@tlrx

This comment has been minimized.

Copy link
Member

commented Jan 9, 2015

Looks like we have a similar behavior for _recovery endpoint:

DELETE /_all

POST /movies/dvd/1
{
"title": "Elasticsearch: the movie"
}

POST /library/book/1
{
"title": "Elasticsearch: the definitve guide"
}

GET /v*/_mapping

returns empty object

GET /v*/_recovery

returns full list

@jpountz jpountz added help wanted and removed discuss labels Jan 9, 2015
@jpountz

This comment has been minimized.

Copy link
Contributor

commented Jan 9, 2015

I just made it an adoptme.

@bleskes

This comment has been minimized.

Copy link
Member

commented Jan 9, 2015

I did some research here. The problem seems to lie in a collision between APIs of lower components. The MetaData#concreteIndices() method takes a list of wildcards/aliases/index names and resolves it to a concrete list of indices to operate on where an empty return means nothing is to be done. The _status API (which is deprecated btw) and the _recovery API both pass this list to RoutingTable#allAssignedShardsGrouped which interprets an empty list to mean all of the indices. Gut feeling says the latter should change and return no shards (because it was probably not updated when the IndexOptions mechanics were introduced) but it's a deep component - we need research the implications.

@cbuescher

This comment has been minimized.

Copy link
Member

commented Jan 23, 2015

Starting to look into this one. Since I'm not familiar with the _recovery api so far I started there and just saw that from the example about also

GET /vfoobar/_recovery?ignore_unavailable=true
returns the full list of indices.

@cbuescher

This comment has been minimized.

Copy link
Member

commented Feb 24, 2015

Removing the expansion of empty list of indices in RoutingTable#allAssignedShardsGrouped solves the problem for the _recovery endpoint, but I'm not sure what the implications are for other operations using that method. From what I see so far these are

  • TransportRefreshAction
  • TransportIndicesStatsAction

Not sure if the semantics of "empty indices" can mean "all indices" in these cases.

An alternative on changing the RoutingTable implementaion would be to set always set the RecoveryRequest "allowNoIndices" option to false. In that case when after expanding wildcards no index is matched we throw an IndexMissingException in MetaData#convertFromWildcards.

There are also at least four other places in RoutingTable where empty index lists are expanded like:

if (indices == null || indices.length == 0) {
            indices = indicesRouting.keySet().toArray(new String[indicesRouting.keySet().size()]);
}

Not sure if all of these should be looked at. I experimentally tried removing all of them but got some test failures after that, so it seems to depend on context when empty index list as input should be treated as "all" or "none".

@cbuescher

This comment has been minimized.

Copy link
Member

commented Feb 27, 2015

I had a closer look at how the index wildcard pattern get resolved to concrete indices in MetaData#concreteIndices().
If it would be 100% sure that any input that could means "all indices" (that is: null or empty String[] or "_all") resolves to concrete list of names, then any empty list that is returned from that method would actually mean that there actually really is no indices or aliases for that input. Then in Routingtable we could get rid of handling the cases where indices=null / empty are treated like "all".
I think that is almost the case already. If wildcard-expansion is on (for open or closed) then the "_all" case is sure to be resolved in MetaData#concreteIndices(). I think there are some edge cases where MetaData#concreteIndices() with empty input index names can lead to NPEs (when both expand_open, expand_closed) are false. Not sure if those cases should be allowed at all?

@cbuescher

This comment has been minimized.

Copy link
Member

commented Feb 27, 2015

I pushed a version where I removed the empty list expansion from the RoutingTable to cbuescher@5f52f9b
Fixed all tests to pass there, not sure if I should open a PR for discussion?

@s1monw s1monw added v1.6.0 and removed v1.5.0 labels Mar 17, 2015
cbuescher added a commit to cbuescher/elasticsearch that referenced this issue Mar 18, 2015
RoutingTables activePrimaryShardsGrouped(), allActiveShardsGrouped() and
allAssignedShardsGrouped() methods treated empty index array input
parameters as meaning "all" indices and expanded to the routing maps
keyset. However, the expansion of index names is now already done in
MetaData#concreteIndices(). Returning an empty index name list here
when a wildcard pattern didn't match any index name could lead to
problems like elastic#9081 because the RoutingTable still expanded this
list of names to "_all". In case of e.g. the recovery endpoint this
could lead to problems.

This fix removes the index name expansion in RoutingTable and introduces
some more checks for preventing NPEs in MetaData#concreteIndices().

Closes elastic#9081
@cbuescher cbuescher closed this in 2beda39 Apr 2, 2015
cbuescher added a commit that referenced this issue Apr 2, 2015
RoutingTables activePrimaryShardsGrouped(), allActiveShardsGrouped() and
allAssignedShardsGrouped() methods treated empty index array input
parameters as meaning "all" indices and expanded to the routing maps
keyset. However, the expansion of index names is now already done in
MetaData#concreteIndices(). Returning an empty index name list here
when a wildcard pattern didn't match any index name could lead to
problems like #9081 because the RoutingTable still expanded this
list of names to "_all". In case of e.g. the recovery endpoint this
could lead to problems.

Closes #9081
Closes #10148
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.