search

blevesearch / zapx

Zap file format compatible with a future version of Bleve
Apache License 2.0
11 stars 12 forks source link

MB-61029: Limiting the time cache remains in memory without a query hit #235

Closed Likith101 closed 5 months ago

abhinavdangeti commented 5 months ago

Looks ok to me, @Thejas-bhat this look ok to you too?

Thejas-bhat commented 5 months ago

I understand the logic behind the changes, however my concern/doubt is what kind of workload patterns would we see the cache entry being cleared - and leading in fresh read the next query. So, before merging maybe running some throughput perf tests under varying workload pattern and figuring out if there is a drop in perf makes more sense to me.

CascadingRadium commented 5 months ago

hey so MB-61650 is fixed with https://github.com/blevesearch/go-faiss/pull/22

Based on my long running throughput experiments i did not observe the case where the index might stay in cache forever, sorry for the confusion there.

Likith101 commented 5 months ago

I calculated the amount of time a cached entry stays in memory vs the qps and these are the results.

Seconds QPS
2 1.5
3 3.7
4 9.3
5 23.4
6 58.5
7 146.4
8 366.2
9 915,5
10 2288.8

So we need 58 qps for a cached entry to be in memory for 6 seconds. With this, I do not this it is necessary to even limit the average value because the cache staying for more than 8-9 seconds is unlikely. cc: @Thejas-bhat @abhinavdangeti

abhinavdangeti commented 5 months ago

Agreed @Likith101 - would you capture this table in the description of the original ticket as well.