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

disable _all field for form, case and sms indexes #25666

Closed
wants to merge 1 commit into from

Conversation

snopoke
Copy link
Contributor

@snopoke snopoke commented Oct 18, 2019

SUMMARY

This removes the _all field form case, form and SMS indexes. This will stop us from using query_string queries on these indexes but we do not use that for these anyway. We DO use query_string queries from user and group indexes so I've left the _all field on those indexes.

When this was applied to the case_search index it reduced the size from 2.26GiB to 1.69GiB.

ES docs: https://www.elastic.co/guide/en/elasticsearch/reference/1.7/mapping-all-field.html#mapping-all-field

@snopoke snopoke added the product/invisible Change has no end-user visible impact label Oct 18, 2019
@emord
Copy link
Contributor

emord commented Oct 18, 2019

Does this have any effect before we re-index?

@dannyroberts
Copy link
Member

dannyroberts commented Oct 18, 2019

I had essentially the same question as Emord; I'm used to seeing us change the index name, e.g.

- CASE_INDEX = es_index("hqcases_2016-03-04")
+ CASE_INDEX = es_index("hqcases_2019-10-18")

so that the change also triggers a full reindex. Is there an alternate workflow/convention for pushing out an "optional" change like this and then allowing each team to reindex their environments manually if and when desired?

Also overall +1 to this change. I'm a little squeemish about how we're know we for sure aren't using this feature, but also trust you must have found a way to make sure of that. Aside from that, I'm excited to reduce the unneeded indexing overhead.

@esoergel
Copy link
Contributor

👍 to the change.

Regarding the existing use of _all, I think it'd be preferable to explicitly specify search fields, and let query string queries search within that whitelist. It really only makes sense to let users search a small subset of the mapped fields, particularly since many aren't intended to be user accessible. I know ES lets you specify a default set of fields to search, but I'm not sure it lets you restrict the fields searched in a "field:value" query string query. That's probably well out of scope for this PR, of course.

@snopoke
Copy link
Contributor Author

snopoke commented Oct 22, 2019

😢 @dannyroberts it would appear you are correct and you can't disable the _all field on an existing index.

In that case this will have to wait until we've upgraded ES.

Also note that from v6.0 the _all field is deprecated: https://www.elastic.co/guide/en/elasticsearch/reference/6.7/mapping-all-field.html#mapping-all-field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product/invisible Change has no end-user visible impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants