Created
September 21, 2018 15:36
-
-
Save umeshdangat/b02e7333513b230d3c2e734e5954a42a to your computer and use it in GitHub Desktop.
Slack Discusion
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| dcausse [8:05 AM] | |
| @umesh do you use ScriptFeature in the scenario where the query cache fails? This is the only query in the ltr plugin that may perform a hashCode/equals on a mutable object. | |
| Also when you said you returned false from isCacheable() you also had to do it from the ConstantScoreWeight returned from RankerQuery::createWeight ? | |
| For me the scenario for this to happen would be: | |
| you use named filters to do logging (using _name = 'ltr_query_log' on sltr) under a filter, this will force the query to be cached | |
| when logging it'll reuse the same Query object but will populate the FeatureSupplier in LtrScript query causing hashCode to change (edited) | |
| umesh [8:15 AM] | |
| do you use ScriptFeature in the scenario where the query cache fails? yes | |
| Also when you said you returned false from isCacheable() you also had to do it from the ConstantScoreWeight returned from RankerQuery::createWeight ? yes | |
| @dcausse the scenario you mention is accurate | |
| dcausse [8:20 AM] | |
| thanks! I'll rework a bit how we share the feature vector (fixing the bug we see on DerivedExpression when requesting the profile api) | |
| this should fix LtrScript as well | |
| umesh [8:21 AM] | |
| @dcausse is it okay to make `isCacheable` return false? | |
| would you anticipate any performance implications? | |
| dcausse [8:21 AM] | |
| I doubt so, I'm OK merging this as a quick fix | |
| umesh [8:22 AM] | |
| ill PR this today | |
| dcausse [8:22 AM] | |
| thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment