Add postings list cache to M3DB index #1370
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1370 +/- ##
========================================
+ Coverage 70.6% 70.7% +0.1%
========================================
Files 823 826 +3
Lines 70369 70663 +294
========================================
+ Hits 49692 49973 +281
- Misses 17455 17459 +4
- Partials 3222 3231 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1370 +/- ##
========================================
+ Coverage 70.7% 70.7% +<.1%
========================================
Files 823 826 +3
Lines 70484 70774 +290
========================================
+ Hits 49840 50081 +241
- Misses 17415 17449 +34
- Partials 3229 3244 +15
Continue to review full report at Codecov.
|
| pattern string, | ||
| patternType PatternType, | ||
| ) (postings.List, bool) { | ||
| // No RLock because a Get() operation mutates the LRU. |
There was a problem hiding this comment.
what's the contention on this thing like? could do a bunch of things to make it less expensive (e.g. approximate the lru, sample updates, etc)
Fine to do this as a follow up too.
There was a problem hiding this comment.
doesn't show up in flame graphs at all, and at the load we're currently handling I don't think its highly contended (100s of queries/s at peak, maybe looking back 100 blocks each is 10,000 locks per second which isn't much). Would rather keep implementation simple (already kind of sucks I had to specialize it) until it becomes an issue which I don't think will happen until we have 1-2 orders of magnitude improvement in general query performance
There was a problem hiding this comment.
wouldn't show up in the cpu flamegraphs necessarily - did you try the blocking?.
mainly curious if this is the cause of latency spikes you mentioned but we can investigate this separately. don't want to block the PR.
There was a problem hiding this comment.
ah...no I did not check blocking. I will get a blocking profile next time I do benchmarking and see if this is an issue. My guess is that its not the root cause of the latency spikes but I'd love to be wrong as this would be an easy fix lol
There was a problem hiding this comment.
Added an additional bullet point to #1365 to track
richardartoul
force-pushed
the
ra/query-cache
branch
from
401abc7 to
6957750
Compare
richardartoul
force-pushed
the
ra/query-cache
branch
from
0d00714 to
9c70c51
Compare
| // NoFinalize marks the Results such that a subsequent call to Finalize() will | ||
| // be a no-op and will not return the object to the pool or release any of its | ||
| // resources. | ||
| NoFinalize() |
There was a problem hiding this comment.
this is only used in tests afaict, can you remove it?
There was a problem hiding this comment.
synced with Rob offline, since its already written and tested gonna leave it in so we can use for the query cache
No description provided.