Skip to content

Update index_options to fix advanced search queries

Dmitry Gruzd requested to merge 213265-fix-search-for-complex-queries into master

What does this MR do?

We recently introduced a regression in !25992 (merged) . This MR changed some index settings in Elasticsearch and it turns out (although not well documented in https://www.elastic.co/guide/en/elasticsearch/reference/current/index-options.html) that if you do use these cheaper index settings then certain "phrase searches" will fail.

In the Elasticsearch docs it gets a casual mention:

Positions can be used for proximity or phrase queries.

But really it means to say that if you don't have positions then any attempt to do a phrase query will cause an error. These errors can be found at https://sentry.gitlab.net/gitlab/gitlabcom/issues/1468715/?query=is%3Aunresolved .

GitLab supports these phrase searches as documented in https://docs.gitlab.com/ee/user/search/advanced_search_syntax.html#using-the-advanced-syntax-search using the Elasticsearch simple query string syntax. As such any users searching for phrases like "hello world" would get a 500.

Until this regression is fixed we've disabled Elasticsearch searching on GitLab.com.

This MR also introduces some more regression tests that we should have had to begin with to test all the examples from our docs across all the different scopes you can search. These tests do correctly fail without the offsets index revert in this MR so this gives us confidence that this problem is fixed.

After this MR is merged we will sadly need to reindex everything in Elasticsearch since there is no way to migrate the schema changes.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Closes #213265 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports