add findCache for memoryIdx #1233
Conversation
Cache patterns and their results in a LRU. When new defs are added to the index, only cached patterns that match the new series name are invalidated. The cache is purged after every prune task runs and after any call to delete items from the index.
woodsaj
force-pushed
the
findCache
branch
3 times, most recently
from
8e3f270 to
2c70e84
Compare
When new series are recieved we need to invlidate any findCache patterns that match the series name. If there are lots of new series being created, this can have a negative performance impact. This change causes the cache to be disabled for 1minute when the rate of new series is higher than what we can process.
- add config settings for:
- find-cache-invalidate-queue: number of new series names to process
Each new series name is compared against expresions in the findCache
if the expression matches the new name, it is removed from the cache.
- find-cache-backoff: amount of time to disable the findCache for when
the invalidate-queue fills up. If there are lots of new series being
added then scanning the cache for patterns that match can become
exspensive, so it is just best to disable the cache for a while.
- log a message when the cache is disabled due to the invalidateQueue
limit being reached.
|
Over the past few weeks we've been trying to make MT use less memory, while sacrificing a bit more CPU for that (f.e. object interning). This change is going into the opposite direction again (more mem / less cpu), so maybe we should disable the find cache by default and only enable it when we know we need it? |
| findCacheMiss.Inc() | ||
| return nil, false | ||
| } | ||
| nodes, ok := cache.Get(pattern) |
There was a problem hiding this comment.
Is there a chance that cache could have already been deleted before attempting this call in some fringe situations?
Edit:
I think all of the calls are wrapped inside RLocks or Locks in the index.
There was a problem hiding this comment.
It is possible that one thread calls cache, ok := c.cache[orgId] then releases the lock, then before nodes, ok := cache.Get(pattern) is executed another thread gets a write lock and calls delete(c.cache, orgId). But it doesnt really matter. This is still safe as the items in c.cache[orgId] are pointers. The end result is that calls to findCache.Get() will return results based on the content of cache when the Rlock() was acquired an not on the contents of the cache at the specific time that nodes, ok := cache.Get(pattern) is executed.
Co-Authored-By: woodsaj <[email protected]>
It is more memory, but not much, as the cache is just a map of strings, that point to slices of pointers. Also, though heap memory could be potentially higher, instances with moderate query loads will see a reduction in allocations (which are needed to search the tree) which will help keep RSS lower. |
|
+1 to what Robert said. |
|
Comparison between master and this branch for relevant Benchmarks |
|
this needs more comments about the design and implementation. |
|
Can we add a fast-path for the find-cache disabled case? so it immediately returns out of Add/Purge/.. calls when disabled? |
we already do that. Caches are per orgId, of |
this doesn't seem to be true. Add() will simply add the new cache if it doesn't exist yet, so any subsequent calls will see a cache and operate on it. |
|
|
Not sure what you're trying to say here. |
If the cache is disabled, |
|
future proofing the above URL's: metrictank/idx/memory/find_cache.go Lines 76 to 79 in 707e6ce metrictank/idx/memory/find_cache.go Line 138 in 707e6ce |
|
we cleared it up in a call: the cache cannot be disabled. |
Cache patterns and their results in a LRU.
When new defs are added to the index, only cached patterns that match
the new series name are invalidated. The cache is purged after every
prune task runs and after any call to delete items from the index.