Remove terms filter lookup cache. #9056
Conversation
This is our only cache which is not 'exact' and might allow for stalled results. Additionally, a similar cache that we have and needs to perform lookups in other indices in order to run queries is the script index, and for this index we rely on the filesystem cache, so we should probably do the same with terms filters lookups.
| final GetResponse getResponse = client.get(new GetRequest(lookup.getIndex(), lookup.getType(), lookup.getId()).preference("_local").routing(lookup.getRouting())).actionGet(); | ||
| if (getResponse.isExists()) { | ||
| List<Object> values = XContentMapValues.extractRawValues(lookup.getPath(), getResponse.getSourceAsMap()); | ||
| terms.addAll(values); | ||
| } |
There was a problem hiding this comment.
hmm what do we do if isExists() returns false?
There was a problem hiding this comment.
I know it might be a preexisting issue but if we execute this on a network thread we might deadlock?
There was a problem hiding this comment.
what do we do if isExists() returns false?
If there is no such entry in the index, then we will call the terms filter parser on an empty list, and this generates a MatchNoDocsFilter (see AbstractFieldMapper.termsFilter).
I know it might be a preexisting issue but if we execute this on a network thread we might deadlock?
I did not change the way that the request is performed, so if there is an issue, it is pre-existing indeed. Any suggestion on how to fix the issue (I don't know much about how we deal with thread pools and network threads)? If it's not easy to fix, I'd lean towards opening another issue for it if you don't mind?
There was a problem hiding this comment.
yeah it might be ok to do this since we are only parsing it on a search thread. I wonder if we can add an assertion for the threadpool that it is not a nework thread ever? but opening a diff issue is good
|
left two comments - LGTM otherwise |
jpountz
changed the title
clintongormley
changed the title
clintongormley
added
the
:Search/Search
This is our only cache which is not 'exact' and might allow for stalled results.
Additionally, a similar cache that we have and needs to perform lookups in other
indices in order to run queries is the script index, and for this index we rely
on the filesystem cache, so we should probably do the same with terms filters
lookups.