Include solr query string in cache key #396
Conversation
| @@ -54,7 +54,7 @@ case class AssetFinder( | |||
| updatedAfter.map(t => "updatedAfter" -> Formatter.dateFormat(t)) :: | |||
| updatedBefore.map(t => "updatedBefore" -> Formatter.dateFormat(t)) :: | |||
| state.map(s => "state" -> s.name) :: | |||
| query.map { q => "query" -> "UHOH!!!!" } :: //FIXME: need toCQL traversal | |||
|
💅 game on fleek @defect! |
| @@ -3,4 +3,5 @@ multicollins { | |||
| instanceAssetType = DATA_CENTER | |||
| locationAttribute = LOCATION | |||
| thisInstance = NONE | |||
| queryTimeout = 30 | |||
There was a problem hiding this comment.
Why is this value queryTimeout but the config class is referencing queryCacheTimeout?
There was a problem hiding this comment.
Because i'm sloppy :) Fix incoming.
defect
force-pushed
the
felix-remote_query_cache
branch
from
10bfffd to
6ff36d8
Compare
defect
force-pushed
the
felix-remote_query_cache
branch
from
6ff36d8 to
74b2e80
Compare
|
LGTM - @defect please also be sure to make a PR to update the multicollins parameters docs. https://github.com/tumblr/collins/blob/gh-pages/_includes/configuration/multicollins.html#L23-51 |
|
@Primer42 #399 opened |
|
Do documentation changes go in a different PR? How about updated tests? |
|
Yes, documentation goes in a different PR (#399) since those are pulled in to another branch (gh-pages). I also added a test to make sure the cql query gets included. |
|
LGTM. What a find |
Include solr query string in cache key
|
👍 |
When doing queries across many collins instances (using multi-collins) we cache the response from the remote instances for 30 seconds. The problem is that we do not include the CQL query in the key which can result in two different queries returning the same thing.
This PR changes the behavior by adding the solr query string to the cache key, which should eliminate the problem. I'm "unprotecting"
traverseQueryString()but I think that should be fine as the method doesn't modify or remove anything, it just traverses the solr expression and builds the query string.@tumblr/systems rfr