From f742937359d7c5fd76d2dc9d65b602aa70d865b2 Mon Sep 17 00:00:00 2001 From: andres-portainer <91705312+andres-portainer@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:13:54 -0300 Subject: [PATCH] fix(endpoints): optimize the search performance BE-11267 (#12262) --- .../endpointgroups/endpointgroup_update.go | 4 +- api/http/handler/endpoints/filter.go | 45 ++++----- api/http/handler/endpoints/filter_test.go | 98 +++++++++++++++++++ api/internal/edge/edgegroup.go | 7 +- api/tag/tag.go | 53 +++++----- api/tag/tag_match.go | 14 ++- api/tag/tag_match_test.go | 34 +++---- api/tag/tag_test.go | 34 +++---- 8 files changed, 191 insertions(+), 98 deletions(-) diff --git a/api/http/handler/endpointgroups/endpointgroup_update.go b/api/http/handler/endpointgroups/endpointgroup_update.go index 19c68658d..b50af9044 100644 --- a/api/http/handler/endpointgroups/endpointgroup_update.go +++ b/api/http/handler/endpointgroups/endpointgroup_update.go @@ -98,8 +98,8 @@ func (handler *Handler) updateEndpointGroup(tx dataservices.DataStoreTx, endpoin payloadTagSet := tag.Set(payload.TagIDs) endpointGroupTagSet := tag.Set((endpointGroup.TagIDs)) union := tag.Union(payloadTagSet, endpointGroupTagSet) - intersection := tag.Intersection(payloadTagSet, endpointGroupTagSet) - tagsChanged = len(union) > len(intersection) + intersection := tag.IntersectionCount(payloadTagSet, endpointGroupTagSet) + tagsChanged = len(union) > intersection if tagsChanged { removeTags := tag.Difference(endpointGroupTagSet, payloadTagSet) diff --git a/api/http/handler/endpoints/filter.go b/api/http/handler/endpoints/filter.go index ee2029aa2..96fc53198 100644 --- a/api/http/handler/endpoints/filter.go +++ b/api/http/handler/endpoints/filter.go @@ -193,7 +193,7 @@ func (handler *Handler) filterEndpointsByQuery( return nil, 0, errors.WithMessage(err, "Unable to retrieve tags from the database") } - tagsMap := make(map[portainer.TagID]string) + tagsMap := make(map[portainer.TagID]string, len(tags)) for _, tag := range tags { tagsMap[tag.ID] = tag.Name } @@ -304,8 +304,7 @@ func filterEndpointsBySearchCriteria( ) []portainer.Endpoint { n := 0 for _, endpoint := range endpoints { - endpointTags := convertTagIDsToTags(tagsMap, endpoint.TagIDs) - if endpointMatchSearchCriteria(&endpoint, endpointTags, searchCriteria) { + if endpointMatchSearchCriteria(&endpoint, tagsMap, searchCriteria) { endpoints[n] = endpoint n++ @@ -319,7 +318,7 @@ func filterEndpointsBySearchCriteria( continue } - if edgeGroupMatchSearchCriteria(&endpoint, edgeGroups, searchCriteria, endpoints, endpointGroups) { + if edgeGroupMatchSearchCriteria(&endpoint, edgeGroups, searchCriteria, endpointGroups) { endpoints[n] = endpoint n++ @@ -365,7 +364,7 @@ func filterEndpointsByStatuses(endpoints []portainer.Endpoint, statuses []portai return endpoints[:n] } -func endpointMatchSearchCriteria(endpoint *portainer.Endpoint, tags []string, searchCriteria string) bool { +func endpointMatchSearchCriteria(endpoint *portainer.Endpoint, tagsMap map[portainer.TagID]string, searchCriteria string) bool { if strings.Contains(strings.ToLower(endpoint.Name), searchCriteria) { return true } @@ -380,8 +379,8 @@ func endpointMatchSearchCriteria(endpoint *portainer.Endpoint, tags []string, se return true } - for _, tag := range tags { - if strings.Contains(strings.ToLower(tag), searchCriteria) { + for _, tagID := range endpoint.TagIDs { + if strings.Contains(strings.ToLower(tagsMap[tagID]), searchCriteria) { return true } } @@ -391,16 +390,17 @@ func endpointMatchSearchCriteria(endpoint *portainer.Endpoint, tags []string, se func endpointGroupMatchSearchCriteria(endpoint *portainer.Endpoint, endpointGroups []portainer.EndpointGroup, tagsMap map[portainer.TagID]string, searchCriteria string) bool { for _, group := range endpointGroups { - if group.ID == endpoint.GroupID { - if strings.Contains(strings.ToLower(group.Name), searchCriteria) { - return true - } + if group.ID != endpoint.GroupID { + continue + } - tags := convertTagIDsToTags(tagsMap, group.TagIDs) - for _, tag := range tags { - if strings.Contains(strings.ToLower(tag), searchCriteria) { - return true - } + if strings.Contains(strings.ToLower(group.Name), searchCriteria) { + return true + } + + for _, tagID := range group.TagIDs { + if strings.Contains(strings.ToLower(tagsMap[tagID]), searchCriteria) { + return true } } } @@ -413,11 +413,10 @@ func edgeGroupMatchSearchCriteria( endpoint *portainer.Endpoint, edgeGroups []portainer.EdgeGroup, searchCriteria string, - endpoints []portainer.Endpoint, endpointGroups []portainer.EndpointGroup, ) bool { for _, edgeGroup := range edgeGroups { - relatedEndpointIDs := edge.EdgeGroupRelatedEndpoints(&edgeGroup, endpoints, endpointGroups) + relatedEndpointIDs := edge.EdgeGroupRelatedEndpoints(&edgeGroup, []portainer.Endpoint{*endpoint}, endpointGroups) for _, endpointID := range relatedEndpointIDs { if endpointID == endpoint.ID { @@ -448,16 +447,6 @@ func filterEndpointsByTypes(endpoints []portainer.Endpoint, endpointTypes []port return endpoints[:n] } -func convertTagIDsToTags(tagsMap map[portainer.TagID]string, tagIDs []portainer.TagID) []string { - tags := make([]string, 0, len(tagIDs)) - - for _, tagID := range tagIDs { - tags = append(tags, tagsMap[tagID]) - } - - return tags -} - func filteredEndpointsByTags(endpoints []portainer.Endpoint, tagIDs []portainer.TagID, endpointGroups []portainer.EndpointGroup, partialMatch bool) []portainer.Endpoint { n := 0 for _, endpoint := range endpoints { diff --git a/api/http/handler/endpoints/filter_test.go b/api/http/handler/endpoints/filter_test.go index fda61acee..f19d0a276 100644 --- a/api/http/handler/endpoints/filter_test.go +++ b/api/http/handler/endpoints/filter_test.go @@ -1,6 +1,7 @@ package endpoints import ( + "strconv" "testing" portainer "github.com/portainer/portainer/api" @@ -148,6 +149,103 @@ func Test_Filter_excludeIDs(t *testing.T) { runTests(tests, t, handler, environments) } +func BenchmarkFilterEndpointsBySearchCriteria_PartialMatch(b *testing.B) { + n := 10000 + + endpointIDs := []portainer.EndpointID{} + + endpoints := []portainer.Endpoint{} + for i := range n { + endpoints = append(endpoints, portainer.Endpoint{ + ID: portainer.EndpointID(i + 1), + Name: "endpoint-" + strconv.Itoa(i+1), + GroupID: 1, + TagIDs: []portainer.TagID{1}, + Type: portainer.EdgeAgentOnDockerEnvironment, + }) + + endpointIDs = append(endpointIDs, portainer.EndpointID(i+1)) + } + + endpointGroups := []portainer.EndpointGroup{} + + edgeGroups := []portainer.EdgeGroup{} + for i := range 1000 { + edgeGroups = append(edgeGroups, portainer.EdgeGroup{ + ID: portainer.EdgeGroupID(i + 1), + Name: "edge-group-" + strconv.Itoa(i+1), + Endpoints: append([]portainer.EndpointID{}, endpointIDs...), + Dynamic: true, + TagIDs: []portainer.TagID{1, 2, 3}, + PartialMatch: true, + }) + } + + tagsMap := map[portainer.TagID]string{} + for i := range 10 { + tagsMap[portainer.TagID(i+1)] = "tag-" + strconv.Itoa(i+1) + } + + searchString := "edge-group" + + b.ResetTimer() + + for range b.N { + e := filterEndpointsBySearchCriteria(endpoints, endpointGroups, edgeGroups, tagsMap, searchString) + if len(e) != n { + b.FailNow() + } + } +} + +func BenchmarkFilterEndpointsBySearchCriteria_FullMatch(b *testing.B) { + n := 10000 + + endpointIDs := []portainer.EndpointID{} + + endpoints := []portainer.Endpoint{} + for i := range n { + endpoints = append(endpoints, portainer.Endpoint{ + ID: portainer.EndpointID(i + 1), + Name: "endpoint-" + strconv.Itoa(i+1), + GroupID: 1, + TagIDs: []portainer.TagID{1, 2, 3}, + Type: portainer.EdgeAgentOnDockerEnvironment, + }) + + endpointIDs = append(endpointIDs, portainer.EndpointID(i+1)) + } + + endpointGroups := []portainer.EndpointGroup{} + + edgeGroups := []portainer.EdgeGroup{} + for i := range 1000 { + edgeGroups = append(edgeGroups, portainer.EdgeGroup{ + ID: portainer.EdgeGroupID(i + 1), + Name: "edge-group-" + strconv.Itoa(i+1), + Endpoints: append([]portainer.EndpointID{}, endpointIDs...), + Dynamic: true, + TagIDs: []portainer.TagID{1}, + }) + } + + tagsMap := map[portainer.TagID]string{} + for i := range 10 { + tagsMap[portainer.TagID(i+1)] = "tag-" + strconv.Itoa(i+1) + } + + searchString := "edge-group" + + b.ResetTimer() + + for range b.N { + e := filterEndpointsBySearchCriteria(endpoints, endpointGroups, edgeGroups, tagsMap, searchString) + if len(e) != n { + b.FailNow() + } + } +} + func runTests(tests []filterTest, t *testing.T, handler *Handler, endpoints []portainer.Endpoint) { for _, test := range tests { t.Run(test.title, func(t *testing.T) { diff --git a/api/internal/edge/edgegroup.go b/api/internal/edge/edgegroup.go index 519e4fd7a..255f5f55f 100644 --- a/api/internal/edge/edgegroup.go +++ b/api/internal/edge/edgegroup.go @@ -77,6 +77,7 @@ func edgeGroupRelatedToEndpoint(edgeGroup *portainer.EdgeGroup, endpoint *portai return true } } + return false } @@ -84,12 +85,10 @@ func edgeGroupRelatedToEndpoint(edgeGroup *portainer.EdgeGroup, endpoint *portai if endpointGroup.TagIDs != nil { endpointTags = tag.Union(endpointTags, tag.Set(endpointGroup.TagIDs)) } - edgeGroupTags := tag.Set(edgeGroup.TagIDs) if edgeGroup.PartialMatch { - intersection := tag.Intersection(endpointTags, edgeGroupTags) - return len(intersection) != 0 + return tag.PartialMatch(edgeGroup.TagIDs, endpointTags) } - return tag.FullMatch(edgeGroupTags, endpointTags) + return tag.FullMatch(edgeGroup.TagIDs, endpointTags) } diff --git a/api/tag/tag.go b/api/tag/tag.go index 89f52c3aa..3b6d080b6 100644 --- a/api/tag/tag.go +++ b/api/tag/tag.go @@ -1,64 +1,63 @@ package tag -import portainer "github.com/portainer/portainer/api" +import ( + portainer "github.com/portainer/portainer/api" +) -type tagSet map[portainer.TagID]bool +type tagSet map[portainer.TagID]struct{} // Set converts an array of ids to a set func Set(tagIDs []portainer.TagID) tagSet { - set := map[portainer.TagID]bool{} + set := map[portainer.TagID]struct{}{} for _, tagID := range tagIDs { - set[tagID] = true + set[tagID] = struct{}{} } + return set } -// Intersection returns a set intersection of the provided sets -func Intersection(sets ...tagSet) tagSet { - intersection := tagSet{} - if len(sets) == 0 { - return intersection +// IntersectionCount returns the element count of the intersection of the sets +func IntersectionCount(setA, setB tagSet) int { + if len(setA) > len(setB) { + setA, setB = setB, setA } - setA := sets[0] - for tag := range setA { - inAll := true - for _, setB := range sets { - if !setB[tag] { - inAll = false - break - } - } - if inAll { - intersection[tag] = true + count := 0 + + for tag := range setA { + if _, ok := setB[tag]; ok { + count++ } } - return intersection + return count } // Union returns a set union of provided sets func Union(sets ...tagSet) tagSet { union := tagSet{} + for _, set := range sets { for tag := range set { - union[tag] = true + union[tag] = struct{}{} } } + return union } // Contains return true if setA contains setB -func Contains(setA tagSet, setB tagSet) bool { +func Contains(setA tagSet, setB []portainer.TagID) bool { if len(setA) == 0 || len(setB) == 0 { return false } - for tag := range setB { - if !setA[tag] { + for _, tag := range setB { + if _, ok := setA[tag]; !ok { return false } } + return true } @@ -67,8 +66,8 @@ func Difference(setA tagSet, setB tagSet) tagSet { set := tagSet{} for tag := range setA { - if !setB[tag] { - set[tag] = true + if _, ok := setB[tag]; !ok { + set[tag] = struct{}{} } } diff --git a/api/tag/tag_match.go b/api/tag/tag_match.go index fe6226ea0..fadd02df8 100644 --- a/api/tag/tag_match.go +++ b/api/tag/tag_match.go @@ -1,11 +1,19 @@ package tag +import portainer "github.com/portainer/portainer/api" + // FullMatch returns true if environment tags matches all edge group tags -func FullMatch(edgeGroupTags tagSet, environmentTags tagSet) bool { +func FullMatch(edgeGroupTags []portainer.TagID, environmentTags tagSet) bool { return Contains(environmentTags, edgeGroupTags) } // PartialMatch returns true if environment tags matches at least one edge group tag -func PartialMatch(edgeGroupTags tagSet, environmentTags tagSet) bool { - return len(Intersection(edgeGroupTags, environmentTags)) != 0 +func PartialMatch(edgeGroupTags []portainer.TagID, environmentTags tagSet) bool { + for _, tagID := range edgeGroupTags { + if _, ok := environmentTags[tagID]; ok { + return true + } + } + + return false } diff --git a/api/tag/tag_match_test.go b/api/tag/tag_match_test.go index 6a295eb8e..a540ba544 100644 --- a/api/tag/tag_match_test.go +++ b/api/tag/tag_match_test.go @@ -9,49 +9,49 @@ import ( func TestFullMatch(t *testing.T) { cases := []struct { name string - edgeGroupTags tagSet + edgeGroupTags []portainer.TagID environmentTag tagSet expected bool }{ { name: "environment tag partially match edge group tags", - edgeGroupTags: Set([]portainer.TagID{1, 2, 3}), + edgeGroupTags: []portainer.TagID{1, 2, 3}, environmentTag: Set([]portainer.TagID{1, 2}), expected: false, }, { name: "edge group tags equal to environment tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{1, 2}), expected: true, }, { name: "environment tags fully match edge group tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{1, 2, 3}), expected: true, }, { name: "environment tags do not match edge group tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{3, 4}), expected: false, }, { name: "edge group has no tags and environment has tags", - edgeGroupTags: Set([]portainer.TagID{}), + edgeGroupTags: []portainer.TagID{}, environmentTag: Set([]portainer.TagID{1, 2}), expected: false, }, { name: "edge group has tags and environment has no tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{}), expected: false, }, { name: "both edge group and environment have no tags", - edgeGroupTags: Set([]portainer.TagID{}), + edgeGroupTags: []portainer.TagID{}, environmentTag: Set([]portainer.TagID{}), expected: false, }, @@ -70,55 +70,55 @@ func TestFullMatch(t *testing.T) { func TestPartialMatch(t *testing.T) { cases := []struct { name string - edgeGroupTags tagSet + edgeGroupTags []portainer.TagID environmentTag tagSet expected bool }{ { name: "environment tags partially match edge group tags 1", - edgeGroupTags: Set([]portainer.TagID{1, 2, 3}), + edgeGroupTags: []portainer.TagID{1, 2, 3}, environmentTag: Set([]portainer.TagID{1, 2}), expected: true, }, { name: "environment tags partially match edge group tags 2", - edgeGroupTags: Set([]portainer.TagID{1, 2, 3}), + edgeGroupTags: []portainer.TagID{1, 2, 3}, environmentTag: Set([]portainer.TagID{1, 4, 5}), expected: true, }, { name: "edge group tags equal to environment tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{1, 2}), expected: true, }, { name: "environment tags fully match edge group tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{1, 2, 3}), expected: true, }, { name: "environment tags do not match edge group tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{3, 4}), expected: false, }, { name: "edge group has no tags and environment has tags", - edgeGroupTags: Set([]portainer.TagID{}), + edgeGroupTags: []portainer.TagID{}, environmentTag: Set([]portainer.TagID{1, 2}), expected: false, }, { name: "edge group has tags and environment has no tags", - edgeGroupTags: Set([]portainer.TagID{1, 2}), + edgeGroupTags: []portainer.TagID{1, 2}, environmentTag: Set([]portainer.TagID{}), expected: false, }, { name: "both edge group and environment have no tags", - edgeGroupTags: Set([]portainer.TagID{}), + edgeGroupTags: []portainer.TagID{}, environmentTag: Set([]portainer.TagID{}), expected: false, }, diff --git a/api/tag/tag_test.go b/api/tag/tag_test.go index 168adbef8..ea66110e2 100644 --- a/api/tag/tag_test.go +++ b/api/tag/tag_test.go @@ -7,49 +7,49 @@ import ( portainer "github.com/portainer/portainer/api" ) -func TestIntersection(t *testing.T) { +func TestIntersectionCount(t *testing.T) { cases := []struct { name string setA tagSet setB tagSet - expected tagSet + expected int }{ { name: "positive numbers set intersection", setA: Set([]portainer.TagID{1, 2, 3, 4, 5}), setB: Set([]portainer.TagID{4, 5, 6, 7}), - expected: Set([]portainer.TagID{4, 5}), + expected: 2, }, { name: "empty setA intersection", setA: Set([]portainer.TagID{1, 2, 3}), setB: Set([]portainer.TagID{}), - expected: Set([]portainer.TagID{}), + expected: 0, }, { name: "empty setB intersection", setA: Set([]portainer.TagID{}), setB: Set([]portainer.TagID{1, 2, 3}), - expected: Set([]portainer.TagID{}), + expected: 0, }, { name: "no common elements sets intersection", setA: Set([]portainer.TagID{1, 2, 3}), setB: Set([]portainer.TagID{4, 5, 6}), - expected: Set([]portainer.TagID{}), + expected: 0, }, { name: "equal sets intersection", setA: Set([]portainer.TagID{1, 2, 3}), setB: Set([]portainer.TagID{1, 2, 3}), - expected: Set([]portainer.TagID{1, 2, 3}), + expected: 3, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - result := Intersection(tc.setA, tc.setB) - if !reflect.DeepEqual(result, tc.expected) { + result := IntersectionCount(tc.setA, tc.setB) + if result != tc.expected { t.Errorf("Expected %v, got %v", tc.expected, result) } }) @@ -109,49 +109,49 @@ func TestContains(t *testing.T) { cases := []struct { name string setA tagSet - setB tagSet + setB []portainer.TagID expected bool }{ { name: "setA contains setB", setA: Set([]portainer.TagID{1, 2, 3}), - setB: Set([]portainer.TagID{1, 2}), + setB: []portainer.TagID{1, 2}, expected: true, }, { name: "setA equals to setB", setA: Set([]portainer.TagID{1, 2}), - setB: Set([]portainer.TagID{1, 2}), + setB: []portainer.TagID{1, 2}, expected: true, }, { name: "setA contains parts of setB", setA: Set([]portainer.TagID{1, 2}), - setB: Set([]portainer.TagID{1, 2, 3}), + setB: []portainer.TagID{1, 2, 3}, expected: false, }, { name: "setA does not contain setB", setA: Set([]portainer.TagID{1, 2}), - setB: Set([]portainer.TagID{3, 4}), + setB: []portainer.TagID{3, 4}, expected: false, }, { name: "setA is empty and setB is not empty", setA: Set([]portainer.TagID{}), - setB: Set([]portainer.TagID{1, 2}), + setB: []portainer.TagID{1, 2}, expected: false, }, { name: "setA is not empty and setB is empty", setA: Set([]portainer.TagID{1, 2}), - setB: Set([]portainer.TagID{}), + setB: []portainer.TagID{}, expected: false, }, { name: "setA is empty and setB is empty", setA: Set([]portainer.TagID{}), - setB: Set([]portainer.TagID{}), + setB: []portainer.TagID{}, expected: false, }, }