From fd834d89c076d0a6c57d487ab2a7f9d5489c50f1 Mon Sep 17 00:00:00 2001 From: Shaun Crampton Date: Tue, 11 Jul 2017 11:43:41 +0100 Subject: [PATCH] Code review markups. --- routetable/route_table.go | 4 ++++ routetable/route_table_test.go | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/routetable/route_table.go b/routetable/route_table.go index 378240a20d..9a0ebd7c5f 100644 --- a/routetable/route_table.go +++ b/routetable/route_table.go @@ -357,6 +357,10 @@ func (r *RouteTable) syncRoutesForLink(ifaceName string) error { // 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. +// +// It's important to do the conntrack deletions in the background because scanning the conntrack +// table is very slow if there are a lot of entries. Previously, we did the deletion synchronously +// but that led to lengthy Apply() calls on the critical path. func (r *RouteTable) startConntrackDeletion(ipAddr ip.Addr) { log.WithField("ip", ipAddr).Debug("Starting goroutine to delete conntrack entries") done := make(chan struct{}) diff --git a/routetable/route_table_test.go b/routetable/route_table_test.go index 2cabc49ec6..1b7eb15afc 100644 --- a/routetable/route_table_test.go +++ b/routetable/route_table_test.go @@ -116,8 +116,9 @@ var _ = Describe("RouteTable", func() { }) Describe("with a slow conntrack deletion", func() { + const delay = 300 * time.Millisecond BeforeEach(func() { - dataplane.ConntrackSleep = 30 * time.Millisecond + dataplane.ConntrackSleep = delay }) It("should block a route add until conntrack finished", func() { // Initial apply starts a background thread to delete @@ -129,7 +130,7 @@ var _ = Describe("RouteTable", func() { }) start := time.Now() rt.Apply() - Expect(time.Since(start)).To(BeNumerically(">=", 30*time.Millisecond)) + Expect(time.Since(start)).To(BeNumerically(">=", delay)) }) It("should not block an unrelated route add ", func() { // Initial apply starts a background thread to delete @@ -141,7 +142,7 @@ var _ = Describe("RouteTable", func() { }) start := time.Now() rt.Apply() - Expect(time.Since(start)).To(BeNumerically("<", 10*time.Millisecond)) + Expect(time.Since(start)).To(BeNumerically("<", delay/2)) }) })