Limit eviction search to at most 2048 entries - #1046
Conversation
af032c3 to
0a84ac5
Compare
32971d9 to
9a8d9f1
Compare
| let map = if let Some(from) = self.eviction_cursor.take() | ||
| && to_scan < len | ||
| { | ||
| Either::Left(self.map.range(from..)) |
There was a problem hiding this comment.
So we're going to look at SCAN_BUDGET entries starting at the cursor, marking flows as evictable as we find them and then set the cursor to the last evictable entry we found. This raises a few questions
-
The ordering of the underlying btree map is unrelated to evictability, so entries will shuffle around relative to a fixed point cursor as the table churns. Maybe i'm misunderstanding but I could see pathological cases where evictable flows are always shifting away from the scan budget region?
-
Is the ordering of the btree as it exists today meaningful? Should the order be based on evictability in some way such that we go from max scan which will contain a mix of evictables and non-evictables to a max scan of purely evictables (in which case should there even be a max or should we pop until we have no more evictables?). But differently, should the flow table be a priority queue?
There was a problem hiding this comment.
but I could see pathological cases where evictable flows are always shifting away from the scan budget region?
I think so, yeah. I'm not sure what to do on that front. My feeling is that traffic that would do so is updating the last_hit of flows, such that they would never be evicted under a full scan anyway.
Is the ordering of the btree as it exists today meaningful?
It's not meaningful when walking the iterator directly, but the key needs to be just the flow ID (since their Ord property is how BTreeMap provides
I did do some testing as to how valuable a secondary BTreeSet<(Moment, InnerFlowId)> would be for the benefit of the cleanup task, and it can save us a lot of time there. With eviction though that's insufficient -- a SYN-only flow is at max priority after just a few seconds of inactivity, so looking at all flows ordered by timestamp doesn't help us out. Priority itself changes passively based on e.g. flow state -- maybe part of the issue is that the eviction priority traits are needlessly expressive, and we could subdivide them by class. Not sure how we'd keep that up to date, but there might be something in using fixed classes like that.
9a8d9f1 to
6ba2043
Compare
This commit introduces a hard limit to the number of elements that a slowpath traversal of an LFT is willing to check for eviction scores. In practice, walking the entire datastructrure has proven unacceptably expensive, and greatly harms our overall throughput at high load. Each `FlowTable` includes an eviction counter that we use as the starting point for a scan. This is filled in whenever an eviction lookup runs past its budget, if that budget is lower than the total size. This prevents us from rechecking the same entries over and over. There is more work to be done around making sure that we can have better estimates ahead-of-time of likely-evictable flows, but we're not there yet! Flow cleanup is already an expensive operation, such that we need to narrow lock granularity before we can push any pre-prep work into the periodic task. Closes #1041.
6ba2043 to
01252d4
Compare
This commit introduces a hard limit to the number of elements that a
slowpath traversal of an LFT is willing to check for eviction scores. In
practice, walking the entire datastructrure has proven unacceptably
expensive, and greatly harms our overall throughput at high load.
Each
FlowTableincludes an eviction counter that we use as thestarting point for a scan. This is filled in whenever an eviction lookup
runs past its budget, if that budget is lower than the total size. This
prevents us from rechecking the same entries over and over.
There is more work to be done around making sure that we can have better
estimates ahead-of-time of likely-evictable flows, but we're not there
yet! Flow cleanup is already an expensive operation, such that we need
to narrow lock granularity before we can push any pre-prep work into the
periodic task.
Closes #1041.