Do conntrack deletions in the background. #1498
Conversation
Block on pending deletions if IP is reused.
|
@neiljerram, @doublek tested this and found it to be a strict improvement over what we have now in his high-connection-load testing. Please can you review? |
| @@ -347,6 +355,45 @@ func (r *RouteTable) syncRoutesForLink(ifaceName string) error { | |||
| return nil | |||
| } | |||
|
|
|||
| // startConntrackDeletion starts the deletion of conntrack entries for the given CIDR in the background. Pending | |||
| // deletions are tracked in the pendingConntrackCleanups map so we can block waiting for them later. | |||
There was a problem hiding this comment.
I think we need a bit more context somewhere to record why we're making this change to move conntrack deletions into the background. I'd be happy with any of:
- adding to the comment here
- adding to the commit message
- adding more to the PR comments - either inline explanation or a link to an issue where it's explained.
At the moment - unless I missed it - I don't think we have any of those.
There was a problem hiding this comment.
I have added an issue and updated the comment.
routetable/route_table_test.go
Outdated
| }) | ||
| start := time.Now() | ||
| rt.Apply() | ||
| Expect(time.Since(start)).To(BeNumerically("<", 10*time.Millisecond)) |
There was a problem hiding this comment.
This looks fragile if we're running on a really slow test VM. One option would be a factor of 10, as 0.3s would still be nearly unnoticeable. Is there anything better we can do?
There was a problem hiding this comment.
I couldn't think of a better way to interlock the test without quite a bit of plumbing. Let's just up the timer.
|
retest this please |
Do conntrack deletions in the background.
Block on pending deletions if IP is reused. Improves the slow performance observed in https://github.com/projectcalico/felix/issues/1501
Todos
Release Note