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

Search - enable low_level_cancellation by default. #42291

Merged
merged 1 commit into from
May 22, 2019

Conversation

markharwood
Copy link
Contributor

Benchmarking on worst-case queries (max agg on match_all or popular-term query with large index) was not noticeably slower.

Closes #26258

@markharwood markharwood added >enhancement :Search/Search Search-related issues that do not fall into other categories labels May 21, 2019
@markharwood markharwood self-assigned this May 21, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@markharwood
Copy link
Contributor Author

Benchmarks were conducted using Rally with the following settings:

  • nyc_taxis dataset used as a larger-sized index
  • dedicated test hardware was used and reset between each run using
    • cpufreq-set to ensure clock rate wasn't varying
    • usual rally . best-practice for clearing any OS-level caches
    • elasticsearch server restarted
  • Index was pre-built and benchmarks were tested on the same physical index (ensuring same segments etc)
  • Used worst-case scenario queries to stress-test the loops containing low_level_cancellation (LLC) checks (a max agg on the trip_distance field for both match_all queries and match on high-popularity term passenger_count:1). Each of the queries were expensive to run over so much data so we had a warm-up of 20 iterations followed by timed loop for 100 iterations

Despite these settings, the expected slow-down in LLC:on versus LLC:off failed to show up in test results. If anything the LLC:on results were observed to be faster (?!).

Rally_LLC_variations_-_Kibana

Rally_LLC_variations_-_Kibana

It should be noted that even within runs using the same settings there were variable result timings which suggests some environmental issues running repeated tests. Despite this, the trend seems to show that LLC:on is faster than LLC:off which is counter-intuitive.

Conclusion

Ideally we would have clearly demonstrated our expectations that LLC:on added a small but acceptable overhead to search performance. Instead we seem to see a variety of timings which if anything suggest LLC:on makes things faster. This is a little baffling but my assumption is that we should enable it by default and keep an eye on nightly benchmarks.

@markharwood
Copy link
Contributor Author

test this please

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff is simple, but this required lots of testing. Thanks @markharwood for looking into this!

Benchmarking on worst-case queries (max agg on match_all or popular-term query with large index) was not noticeably slower.

Closes elastic#26258
Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the testing! LGTM

@markharwood markharwood merged commit 40beecd into elastic:master May 22, 2019
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Benchmarking on worst-case queries (max agg on match_all or popular-term query with large index) was not noticeably slower.

Closes elastic#26258
markharwood added a commit to markharwood/elasticsearch that referenced this pull request Jun 7, 2019
Benchmarking on worst-case queries (max agg on match_all or popular-term query with large index) was not noticeably slower.

Closes elastic#26258
markharwood added a commit that referenced this pull request Jun 7, 2019
Benchmarking on worst-case queries (max agg on match_all or popular-term query with large index) was not noticeably slower.

Closes #26258
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider enabling low-level search cancellation by default
5 participants