From aaceb84a0f5fc696e3044a780947c9a4818812f1 Mon Sep 17 00:00:00 2001 From: Paul Lorenz Date: Thu, 4 Apr 2024 18:18:35 -0400 Subject: [PATCH] Don't save old link statues in memory, the info is in link events. Fixes #1896 --- controller/api_impl/link_api_model.go | 6 +--- controller/network/assembly.go | 4 +-- controller/network/link.go | 42 +++++++++------------- controller/network/link_controller.go | 13 +++---- controller/network/link_controller_test.go | 2 +- controller/network/network.go | 6 ++-- controller/network/path_test.go | 36 +++++++++---------- controller/network/route_perf_test.go | 8 ++--- 8 files changed, 50 insertions(+), 67 deletions(-) diff --git a/controller/api_impl/link_api_model.go b/controller/api_impl/link_api_model.go index 2059df50d..d8338813b 100644 --- a/controller/api_impl/link_api_model.go +++ b/controller/api_impl/link_api_model.go @@ -44,11 +44,7 @@ func (factory *LinkLinkFactoryIml) Links(entity LinkEntity) rest_model.Links { func MapLinkToRestModel(n *network.Network, _ api.RequestContext, link *network.Link) (*rest_model.LinkDetail, error) { iteration := int64(link.Iteration) staticCost := int64(link.StaticCost) - linkState := link.CurrentState() - linkStateStr := "" - if linkState != nil { - linkStateStr = linkState.Mode.String() - } + linkStateStr := link.CurrentState().Mode.String() down := link.IsDown() diff --git a/controller/network/assembly.go b/controller/network/assembly.go index 0a8bf5213..6093bfe9e 100644 --- a/controller/network/assembly.go +++ b/controller/network/assembly.go @@ -129,10 +129,10 @@ func (network *Network) clean() { failedLinks = append(failedLinks, duplicateLinks...) now := info.NowInMilliseconds() - lRemove := make(map[string]*Link) + var lRemove []*Link for _, l := range failedLinks { if now-l.CurrentState().Timestamp >= 30000 { - lRemove[l.Id] = l + lRemove = append(lRemove, l) } } for _, lr := range lRemove { diff --git a/controller/network/link.go b/controller/network/link.go index db56db3dd..b6c5becb9 100644 --- a/controller/network/link.go +++ b/controller/network/link.go @@ -18,7 +18,6 @@ package network import ( "github.com/openziti/foundation/v2/concurrenz" - "github.com/openziti/foundation/v2/info" "sync" "sync/atomic" "time" @@ -35,7 +34,7 @@ type Link struct { Dst concurrenz.AtomicValue[*Router] Protocol string DialAddress string - state []*LinkState + state LinkState down bool StaticCost int32 usable atomic.Bool @@ -47,14 +46,17 @@ func newLink(id string, linkProtocol string, dialAddress string, initialLatency Id: id, Protocol: linkProtocol, DialAddress: dialAddress, - state: make([]*LinkState, 0), - down: false, - StaticCost: 1, - SrcLatency: initialLatency.Nanoseconds(), - DstLatency: initialLatency.Nanoseconds(), + state: LinkState{ + Mode: Pending, + Timestamp: time.Now().UnixMilli(), + }, + down: false, + StaticCost: 1, + SrcLatency: initialLatency.Nanoseconds(), + DstLatency: initialLatency.Nanoseconds(), } l.recalculateCost() - l.addState(&LinkState{Mode: Pending, Timestamp: info.NowInMilliseconds()}) + l.recalculateUsable() return l } @@ -66,23 +68,18 @@ func (link *Link) GetDest() *Router { return link.Dst.Load() } -func (link *Link) CurrentState() *LinkState { +func (link *Link) CurrentState() LinkState { link.lock.Lock() defer link.lock.Unlock() - if link.state == nil || len(link.state) < 1 { - return nil - } - return link.state[0] + return link.state } -func (link *Link) addState(s *LinkState) { +func (link *Link) SetState(m LinkMode) { link.lock.Lock() defer link.lock.Unlock() - if link.state == nil { - link.state = make([]*LinkState, 0) - } - link.state = append([]*LinkState{s}, link.state...) + link.state.Mode = m + link.state.Timestamp = time.Now().UnixMilli() link.recalculateUsable() } @@ -102,7 +99,7 @@ func (link *Link) IsDown() bool { func (link *Link) recalculateUsable() { if link.down { link.usable.Store(false) - } else if len(link.state) < 1 || link.state[0].Mode != Connected { + } else if link.state.Mode != Connected { link.usable.Store(false) } else { link.usable.Store(true) @@ -176,10 +173,3 @@ type LinkState struct { Mode LinkMode Timestamp int64 } - -func newLinkState(mode LinkMode) *LinkState { - return &LinkState{ - Mode: mode, - Timestamp: info.NowInMilliseconds(), - } -} diff --git a/controller/network/link_controller.go b/controller/network/link_controller.go index 500bb1a21..a0f6f1774 100644 --- a/controller/network/link_controller.go +++ b/controller/network/link_controller.go @@ -90,11 +90,8 @@ func newLinkController(options *Options) *linkController { return &val }) result.store.AddStringSymbol("state", func(entity *Link) *string { - if state := entity.CurrentState(); state != nil { - val := state.Mode.String() - return &val - } - return nil + val := entity.CurrentState().Mode.String() + return &val }) result.store.AddInt64Symbol("iteration", func(entity *Link) *int64 { val := int64(entity.Iteration) @@ -161,7 +158,7 @@ func (linkController *linkController) routerReportedLink(linkId string, iteratio link.Src = src link.Dst.Store(dst) link.DstId = dstId - link.addState(newLinkState(Connected)) + link.SetState(Connected) linkController.add(link) return link, true } @@ -273,7 +270,7 @@ func (linkController *linkController) clearExpiredPending(pendingTimeout time.Du toRemove := linkController.linkTable.matching(func(link *Link) bool { state := link.CurrentState() - return state != nil && state.Mode == Pending && state.Timestamp < pendingLimit + return state.Mode == Pending && state.Timestamp < pendingLimit }) for _, link := range toRemove { @@ -289,7 +286,7 @@ func (linkController *linkController) hasDirectedLink(a, b *Router, linkProtocol links := a.routerLinks.GetLinks() for _, link := range links { state := link.CurrentState() - if link.Src == a && link.DstId == b.Id && state != nil && link.Protocol == linkProtocol { + if link.Src == a && link.DstId == b.Id && link.Protocol == linkProtocol { if state.Mode == Connected || (state.Mode == Pending && state.Timestamp > pendingLimit) { return true } diff --git a/controller/network/link_controller_test.go b/controller/network/link_controller_test.go index 2a91e91a5..b257af6b6 100644 --- a/controller/network/link_controller_test.go +++ b/controller/network/link_controller_test.go @@ -77,7 +77,7 @@ func TestNeighbors(t *testing.T) { r0 := newRouterForTest("r0", "", nil, nil, 0, true) r1 := newRouterForTest("r1", "", nil, nil, 0, true) l0 := newTestLink("l0", r0, r1) - l0.addState(newLinkState(Connected)) + l0.SetState(Connected) linkController.add(l0) neighbors := linkController.connectedNeighborsOfRouter(r0) diff --git a/controller/network/network.go b/controller/network/network.go index f6755a2ce..5336e52f9 100644 --- a/controller/network/network.go +++ b/controller/network/network.go @@ -458,11 +458,11 @@ func (network *Network) NotifyExistingLink(id string, iteration uint32, linkProt func (network *Network) LinkConnected(msg *ctrl_pb.LinkConnected) error { if l, found := network.linkController.get(msg.Id); found { - if state := l.CurrentState(); state != nil && state.Mode != Pending { + if state := l.CurrentState(); state.Mode != Pending { return errors.Errorf("link [l/%v] state is %v, not pending, cannot mark connected", msg.Id, state.Mode) } - l.addState(newLinkState(Connected)) + l.SetState(Connected) network.NotifyLinkConnected(l, msg) return nil } @@ -470,7 +470,7 @@ func (network *Network) LinkConnected(msg *ctrl_pb.LinkConnected) error { } func (network *Network) LinkFaulted(l *Link, dupe bool) error { - l.addState(newLinkState(Failed)) + l.SetState(Failed) if dupe { network.NotifyLinkEvent(l, event.LinkDuplicate) } else { diff --git a/controller/network/path_test.go b/controller/network/path_test.go index 678428c22..144e1b419 100644 --- a/controller/network/path_test.go +++ b/controller/network/path_test.go @@ -51,7 +51,7 @@ func TestSimplePath2(t *testing.T) { network.Routers.markConnected(r1) l0 := newTestLink("l0", r0, r1) - l0.addState(newLinkState(Connected)) + l0.SetState(Connected) network.linkController.add(l0) path, err := network.CreatePath(r0, r1) @@ -115,11 +115,11 @@ func TestTransitPath2(t *testing.T) { network.Routers.markConnected(r2) l0 := newTestLink("l0", r0, r1) - l0.addState(newLinkState(Connected)) + l0.SetState(Connected) network.linkController.add(l0) l1 := newTestLink("l1", r1, r2) - l1.addState(newLinkState(Connected)) + l1.SetState(Connected) network.linkController.add(l1) path, err := network.CreatePath(r0, r2) @@ -218,28 +218,28 @@ func TestShortestPath(t *testing.T) { link.SetStaticCost(2) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l1", r0, r2) link.SetStaticCost(5) link.SetDstLatency(15 * 1_000_000) link.SetSrcLatency(16 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l2", r1, r3) link.SetStaticCost(9) link.SetDstLatency(20 * 1_000_000) link.SetSrcLatency(21 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l3", r2, r3) link.SetStaticCost(13) link.SetDstLatency(25 * 1_000_000) link.SetSrcLatency(26 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) path, cost, err := network.shortestPath(r0, r3) @@ -286,28 +286,28 @@ func TestShortestPathWithUntraversableRouter(t *testing.T) { link.SetStaticCost(2) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l1", r0, r2) link.SetStaticCost(5) link.SetDstLatency(15 * 1_000_000) link.SetSrcLatency(16 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l2", r1, r3) link.SetStaticCost(9) link.SetDstLatency(20 * 1_000_000) link.SetSrcLatency(21 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l3", r2, r3) link.SetStaticCost(13) link.SetDstLatency(25 * 1_000_000) link.SetSrcLatency(26 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) path, cost, err := network.shortestPath(r0, r3) @@ -348,7 +348,7 @@ func TestShortestPathWithOnlyUntraversableRouter(t *testing.T) { link.SetStaticCost(2) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) path, cost, err := network.shortestPath(r0, r1) @@ -388,7 +388,7 @@ func TestShortestPathWithUntraversableEdgeRouters(t *testing.T) { link.SetStaticCost(3) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) path, cost, err := network.shortestPath(r0, r1) @@ -431,14 +431,14 @@ func TestShortestPathWithUntraversableEdgeRoutersAndTraversableMiddle(t *testing link.SetStaticCost(2) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l1", r1, r2) link.SetStaticCost(3) link.SetDstLatency(12 * 1_000_000) link.SetSrcLatency(15 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) path, cost, err := network.shortestPath(r0, r2) @@ -483,14 +483,14 @@ func TestShortestPathWithUntraversableEdgeRoutersAndUntraversableMiddle(t *testi link.SetStaticCost(2) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) link = newTestLink("l2", r1, r2) link.SetStaticCost(2) link.SetDstLatency(10 * 1_000_000) link.SetSrcLatency(11 * 1_000_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) path, cost, err := network.shortestPath(r0, r2) @@ -619,7 +619,7 @@ func newPathTestLink(network *Network, id string, srcR, destR *Router) *Link { l.SrcLatency = 0 l.DstLatency = 0 l.recalculateCost() - l.addState(newLinkState(Connected)) + l.SetState(Connected) network.linkController.add(l) return l } diff --git a/controller/network/route_perf_test.go b/controller/network/route_perf_test.go index c28daf0ef..7f24b2eeb 100644 --- a/controller/network/route_perf_test.go +++ b/controller/network/route_perf_test.go @@ -62,7 +62,7 @@ func TestShortestPathAgainstEstablished(t *testing.T) { link.SetStaticCost(int32(nextCost())) link.SetDstLatency(nextCost() * 100_000) link.SetSrcLatency(nextCost() * 100_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) linkIdx++ } @@ -186,7 +186,7 @@ func BenchmarkShortestPathPerfWithRouterChanges(b *testing.B) { link.SetStaticCost(int32(nextCost())) link.SetDstLatency(nextCost() * 100_000) link.SetSrcLatency(nextCost() * 100_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) linkIdx++ } @@ -278,7 +278,7 @@ func BenchmarkShortestPathPerf(b *testing.B) { link.SetStaticCost(int32(nextCost())) link.SetDstLatency(nextCost() * 100_000) link.SetSrcLatency(nextCost() * 100_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) linkIdx++ } @@ -350,7 +350,7 @@ func BenchmarkMoreRealisticShortestPathPerf(b *testing.B) { link.SetStaticCost(int32(nextCost())) link.SetDstLatency(nextCost() * 100_000) link.SetSrcLatency(nextCost() * 100_000) - link.addState(newLinkState(Connected)) + link.SetState(Connected) network.linkController.add(link) linkIdx++ }