Extend admission policies, statistics, add multi thread eviction and promotion #90
Conversation
igchor
force-pushed
the
in_progress_promotion
branch
from
dec1628 to
5a58a6d
Compare
|
TODO:
|
igchor
force-pushed
the
in_progress_promotion
branch
3 times, most recently
from
401bf23 to
9476e5c
Compare
igchor
force-pushed
the
in_progress_promotion
branch
2 times, most recently
from
1716061 to
66a317f
Compare
|
Hi Igor, currently some monitoring scripts expect the format below. If we use this format the scripts will pick up the metrics. The other format would work as well, we just need to modify some monitor scripts. == Class Background Promotion Counters Map == |
igchor
force-pushed
the
in_progress_promotion
branch
5 times, most recently
from
625c23c to
9cd86c5
Compare
|
Hi @igchor, can we use the same output format for |
|
@raema Sure, I'll change it |
There was a problem hiding this comment.
Reviewable status: 0 of 36 files reviewed, 1 unresolved discussion (waiting on @igchor and @vinser52)
cachelib-background-evictor.png line 0 at r1 (raw file):
As we agreed, we should avoid using PMEM. Let's make the picture more abstract.
There was a problem hiding this comment.
Reviewable status: 0 of 36 files reviewed, 1 unresolved discussion (waiting on @byrnedj and @vinser52)
cachelib-background-evictor.png line at r1 (raw file):
Previously, vinser52 (Sergei Vinogradov) wrote…
As we agreed, we should avoid using PMEM. Let's make the picture more abstract.
@byrnedj could you please fix the picture? I don't think I have access to the source image.
There was a problem hiding this comment.
Reviewable status: 0 of 36 files reviewed, 2 unresolved discussions (waiting on @byrnedj and @vinser52)
-- commits line 12 at r1:
@vinser52 what do you think about this patch? Are you OK with merging this for now and possibly reverting later (once we enable transparent synchronization for chained items)?
Right now, in my patch, we do not use any extra synchronization for items being moved - we just use moveCb (or fallback to memcpy if none is provided). This should be fine for cachebench and Jimmy also said it's fine for them.
There was a problem hiding this comment.
Reviewable status: 0 of 36 files reviewed, 5 unresolved discussions (waiting on @byrnedj, @igchor, and @vinser52)
MultiTierDataMovement.md line 20 at r1 (raw file):
## Background Evictors The background evictors scan each class to see if there are objects to move the next (higher)
Are we talking about eviction or promotion thread here? If it is an eviction thread then "move to the next (higher) tier" is confusing, it should "down" instead of "higher".
Right?
MultiTierDataMovement.md line 39 at r1 (raw file):
- `maxEvictionBatch`: The number of objects to remove in a given eviction call. The default is 40. Lower range is 10 and the upper range is 1000. Too low and we might not remove objects at a reasonable rate, too high and we hold the locks for copying data
I believe that we exposing implementation details with that phrase: "we hold the locks for copying data". Probably better to say that it might increase contention with user threads.
MultiTierDataMovement.md line 47 at r1 (raw file):
- `maxEvictionPromotionHotness`: Maximum candidates to consider for eviction. This is similar to `maxEvictionBatch` but it specifies how many candidates will be taken into consideration, not the actual number of items to evict. This option can be used to configure duration of critical section on LRU lock.
The same as the previous comment. We are exposing implementation details. Let's be less specific.
igchor
force-pushed
the
in_progress_promotion
branch
from
4d177fd to
682cee4
Compare
There was a problem hiding this comment.
@raema Done. Output is now:
== Class Background Eviction Counters Map ==
tid 0 pid 0 cid 1 evicted: 132064
tid 0 pid 0 cid 2 evicted: 332206
tid 0 pid 0 cid 3 evicted: 488053
tid 0 pid 0 cid 4 evicted: 888834
tid 0 pid 0 cid 5 evicted: 777947
tid 0 pid 0 cid 6 evicted: 266416
tid 0 pid 0 cid 7 evicted: 133270
tid 0 pid 0 cid 8 evicted: 111095
tid 0 pid 0 cid 9 evicted: 111005
tid 0 pid 0 cid 10 evicted: 111015
tid 0 pid 0 cid 11 evicted: 111000
tid 0 pid 0 cid 12 evicted: 106050
tid 0 pid 0 cid 13 evicted: 89620
tid 0 pid 0 cid 14 evicted: 89615
tid 0 pid 0 cid 15 evicted: 89635
tid 0 pid 0 cid 16 evicted: 107845
tid 0 pid 0 cid 17 evicted: 84529
tid 0 pid 0 cid 18 evicted: 60725
tid 0 pid 0 cid 19 evicted: 39425
== Class Background Promotion Counters Map ==
tid 1 pid 0 cid 1 promoted: 85
tid 1 pid 0 cid 2 promoted: 156
tid 1 pid 0 cid 3 promoted: 165
tid 1 pid 0 cid 4 promoted: 110
tid 1 pid 0 cid 5 promoted: 115
tid 1 pid 0 cid 6 promoted: 95
tid 1 pid 0 cid 7 promoted: 110
tid 1 pid 0 cid 8 promoted: 150
tid 1 pid 0 cid 9 promoted: 100
tid 1 pid 0 cid 10 promoted: 90
tid 1 pid 0 cid 11 promoted: 125
tid 1 pid 0 cid 12 promoted: 80681
tid 1 pid 0 cid 13 promoted: 81997
tid 1 pid 0 cid 14 promoted: 83309
tid 1 pid 0 cid 15 promoted: 83151
tid 1 pid 0 cid 16 promoted: 76818
tid 1 pid 0 cid 17 promoted: 81311
tid 1 pid 0 cid 18 promoted: 59994
tid 1 pid 0 cid 19 promoted: 39424
Reviewable status: 0 of 36 files reviewed, 5 unresolved discussions (waiting on @byrnedj and @vinser52)
MultiTierDataMovement.md line 20 at r1 (raw file):
Previously, vinser52 (Sergei Vinogradov) wrote…
Are we talking about eviction or promotion thread here? If it is an eviction thread then "move to the next (higher) tier" is confusing, it should "down" instead of "higher".
Right?
Right, something went wrong here. Done.
MultiTierDataMovement.md line 39 at r1 (raw file):
Previously, vinser52 (Sergei Vinogradov) wrote…
I believe that we exposing implementation details with that phrase: "we hold the locks for copying data". Probably better to say that it might increase contention with user threads.
Done.
MultiTierDataMovement.md line 47 at r1 (raw file):
Previously, vinser52 (Sergei Vinogradov) wrote…
The same as the previous comment. We are exposing implementation details. Let's be less specific.
Done.
igchor
force-pushed
the
in_progress_promotion
branch
from
682cee4 to
42e103a
Compare
and add additional parameters to control allocation and eviction of items. Co-authored-by: Daniel Byrne <[email protected]>
instead of relying on transparent synchronization mechanims (wait context).
and add a simple unit test
igchor
force-pushed
the
in_progress_promotion
branch
from
42e103a to
b7300e2
Compare
Hot queue iterator for 2Q. Will start at Hot queue and move to Warm queue if hot queue is exhausted. Useful for promotion semantics if using 2Q replacement. rebased on to develop and added some tests.
igchor
force-pushed
the
in_progress_promotion
branch
from
b7300e2 to
a3e29dd
Compare
Please take a look at https://github.com/pmem/CacheLib/blob/9dbbc6d6cc994312bf7d47bc3a9a174ae6a52bb3/BackgroundMovers.md for eviction description.
Most new config options are here:
CacheLib/cachelib/cachebench/util/CacheConfig.h
Line 297 in 9dbbc6d
This change is