Page MenuHomePhabricator
Create Task
Maniphest T171579

The ltr plugin fails on a circuit breaker because of script.max_compilations_per_minute
Closed, ResolvedPublic
Actions

Description

By reusing the ScriptService we are hitting a circuit breaker that prevents large featuresets to be compiled in one row.
Currently we used featureset with 14 features and the limit is 15 compilations per minutes, with larger featuresets we need to send the query multiple times in several minutes to be able to compile all the features.
This is rather dangerous and we should find a solution:

  • easy/short term: increase script.max_compilations_per_minute so that we never fail on it
  • do not depend on ScriptService: import mustache and do the templating on our own (storing compiled templates in our cache)

Details

SubjectRepoBranchLines +/-
Add script.max_compilations_per_minute to elasticsearchoperations/puppetproduction+33 -21
Customize query in gerrit

Event Timeline

dcausse created this task.Jul 25 2017, 11:54 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 25 2017, 11:54 AM
dcausse added a project: CirrusSearch.Jul 25 2017, 11:54 AM
Restricted Application edited projects, added Discovery-ARCHIVED, Discovery-Search; removed Discovery-Search (Current work). · View Herald TranscriptJul 25 2017, 11:54 AM
gerritbot subscribed.Jul 25 2017, 12:11 PM

Change 367668 had a related patch set uploaded (by DCausse; owner: DCausse):
[operations/puppet@production] Add script.max_compilations_per_minute to elasticsearch

https://gerrit.wikimedia.org/r/367668

gerritbot added a project: Patch-For-Review.Jul 25 2017, 12:11 PM
gerritbot added a comment.Jul 25 2017, 1:50 PM

Change 367668 merged by Gehel:
[operations/puppet@production] Add script.max_compilations_per_minute to elasticsearch

https://gerrit.wikimedia.org/r/367668

debt triaged this task as Medium priority.Jul 27 2017, 5:09 PM
debt edited projects, added Discovery-Search (Current work); removed Discovery-Search.
debt moved this task from Incoming to not in use - please delete on the Discovery-Search (Current work) board.
debt subscribed.

The work-around for this is good, but we probably want a more permanent solution.

EBernhardson assigned this task to dcausse.Jul 27 2017, 6:12 PM
dcausse added a comment.Jul 28 2017, 4:35 PM

simple poc https://github.com/nomoa/elasticsearch-learning-to-rank/tree/embed_mustache
tests pass fine, will test more on real nodes

dcausse moved this task from not in use - please delete to Needs review on the Discovery-Search (Current work) board.Aug 1 2017, 3:07 PM
dcausse added a comment.Aug 1 2017, 4:43 PM

tested on relforge and my local machine (with max_compilations_per_minute set to 4) and seems to work as expected (PR: https://github.com/o19s/elasticsearch-learning-to-rank/pull/64)

EBernhardson moved this task from Needs review to Needs Reporting on the Discovery-Search (Current work) board.Aug 2 2017, 10:34 PM
EBernhardson subscribed.

This is merged, although we will need to deploy the new plugin version.

dcausse added a comment.Aug 3 2017, 9:59 AM

https://gerrit.wikimedia.org/r/#/c/352170 is updated but not yet deployed, I think it's fine to run the A/B on the current deployed version of the plugin:
It has https://github.com/o19s/elasticsearch-learning-to-rank/commits/1_0 up to fdd71b1 which includes all fixes for runtime scoring of xgboost models.
We agreed with Guillaume to deploy the new plugins on codfw first then on eqiad after the interleaving A/B test is done.

debt closed this task as Resolved.Aug 4 2017, 8:17 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL