Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: optimize related_change_recursive_query #1165

Merged
merged 3 commits into from
Nov 1, 2024
Merged

chore: optimize related_change_recursive_query #1165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23f6b54
chore: optimize related_change_recursive_query
yashmehrotra Oct 30, 2024
39c05af
chore: update tests
yashmehrotra Nov 1, 2024
4d065d3
chore: document soft relation problems
yashmehrotra Nov 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion query/config_changes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ type CatalogChangesSearchRequest struct {

// upstream | downstream | both
Recursive string `query:"recursive" json:"recursive"`
// FIXME: Soft toggle does not work with Recursive=both
// In that case, soft relations are always returned
// It also returns ALL soft relations throughout the tree
// not just soft related to the main config item
Soft bool `query:"soft" json:"soft"`

fromParsed time.Time
toParsed time.Time
Expand Down Expand Up @@ -347,7 +352,7 @@ func FindCatalogChanges(ctx context.Context, req CatalogChangesSearchRequest) (*

table := query.Table("catalog_changes")
if err := uuid.Validate(req.CatalogID); err == nil {
table = query.Table("related_changes_recursive(?,?,?,?)", req.CatalogID, req.Recursive, req.IncludeDeletedConfigs, req.Depth)
table = query.Table("related_changes_recursive(?,?,?,?,?)", req.CatalogID, req.Recursive, req.IncludeDeletedConfigs, req.Depth, req.Soft)
} else {
clause, err := parseAndBuildFilteringQuery(req.CatalogID, "config_id", false)
if err != nil {
Expand Down
123 changes: 96 additions & 27 deletions tests/config_changes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tests

import (
"strings"
"time"

"github.com/flanksource/duty/db"
Expand All @@ -17,32 +18,36 @@ import (
var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
// Graph #1 (acyclic)
//
// U
// U --- (A)
// / \
// (B)----V W
// / \
// V W
// / \
// X Y
// /
// Z
// (C)--X Y
// /
// Z--- (D)

// Create a list of ConfigItems
var (
U = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Node"), Name: lo.ToPtr("U"), ConfigClass: "Node"}
V = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Deployment"), Name: lo.ToPtr("V"), ConfigClass: "Deployment"}
W = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("W"), ConfigClass: "Pod"}
X = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ReplicaSet"), Name: lo.ToPtr("X"), ConfigClass: "ReplicaSet"}
Y = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::PersistentVolume"), Name: lo.ToPtr("Y"), ConfigClass: "PersistentVolume"}
Z = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("Z"), ConfigClass: "Pod"}
V = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Deployment"), Name: lo.ToPtr("V"), ConfigClass: "Deployment", ParentID: lo.ToPtr(U.ID), Path: U.ID.String()}
W = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("W"), ConfigClass: "Pod", ParentID: lo.ToPtr(U.ID), Path: U.ID.String()}
X = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ReplicaSet"), Name: lo.ToPtr("X"), ConfigClass: "ReplicaSet", ParentID: lo.ToPtr(V.ID), Path: strings.Join([]string{U.ID.String(), V.ID.String()}, ".")}
Y = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::PersistentVolume"), Name: lo.ToPtr("Y"), ConfigClass: "PersistentVolume", ParentID: lo.ToPtr(V.ID), Path: strings.Join([]string{U.ID.String(), V.ID.String()}, ".")}
Z = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("Z"), ConfigClass: "Pod", ParentID: lo.ToPtr(X.ID), Path: strings.Join([]string{U.ID.String(), V.ID.String(), X.ID.String()}, ".")}

A = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("A"), ConfigClass: "ConfigMap"}
B = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("B"), ConfigClass: "ConfigMap"}
C = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("C"), ConfigClass: "ConfigMap"}
D = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("D"), ConfigClass: "ConfigMap"}
)
configItems := []models.ConfigItem{U, V, W, X, Y, Z}
configItems := []models.ConfigItem{U, V, W, X, Y, Z, A, B, C, D}

// Create relationships between ConfigItems
relationships := []models.ConfigRelationship{
{ConfigID: U.ID.String(), RelatedID: V.ID.String(), Relation: "test-changes-UV"},
{ConfigID: U.ID.String(), RelatedID: W.ID.String(), Relation: "test-changes-UW"},
{ConfigID: V.ID.String(), RelatedID: X.ID.String(), Relation: "test-changes-VX"},
{ConfigID: V.ID.String(), RelatedID: Y.ID.String(), Relation: "test-changes-VY"},
{ConfigID: X.ID.String(), RelatedID: Z.ID.String(), Relation: "test-changes-XZ"},
{ConfigID: U.ID.String(), RelatedID: A.ID.String(), Relation: "test-changes-UA"},
{ConfigID: V.ID.String(), RelatedID: B.ID.String(), Relation: "test-changes-VB"},
{ConfigID: X.ID.String(), RelatedID: C.ID.String(), Relation: "test-changes-XC"},
{ConfigID: Z.ID.String(), RelatedID: D.ID.String(), Relation: "test-changes-ZD"},
}

// Create changes for each config
Expand All @@ -54,7 +59,12 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
YChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 4)), Severity: "warn", ConfigID: Y.ID.String(), Summary: ".name.Y", ChangeType: "diff", Source: "test-changes"}
ZChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: Z.ID.String(), Summary: ".name.Z", ChangeType: "Pulled", Source: "test-changes"}

changes = []models.ConfigChange{UChange, VChange, WChange, XChange, YChange, ZChange}
AChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: A.ID.String(), Summary: ".name.A", ChangeType: "Pulled", Source: "test-changes"}
BChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: B.ID.String(), Summary: ".name.B", ChangeType: "Pulled", Source: "test-changes"}
CChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: C.ID.String(), Summary: ".name.C", ChangeType: "Pulled", Source: "test-changes"}
DChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: D.ID.String(), Summary: ".name.D", ChangeType: "Pulled", Source: "test-changes"}

changes = []models.ConfigChange{UChange, VChange, WChange, XChange, YChange, ZChange, AChange, BChange, CChange, DChange}
)

ginkgo.BeforeAll(func() {
Expand Down Expand Up @@ -111,13 +121,12 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
ginkgo.Context("Both ways", func() {
ginkgo.It("should return changes upstream and downstream", func() {
relatedChanges, err := findChanges(X.ID, "all", false)

Expect(err).To(BeNil())

Expect(len(relatedChanges.Changes)).To(Equal(4))
Expect(len(relatedChanges.Changes)).To(Equal(6))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, XChange.ID, ZChange.ID}))
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, XChange.ID, ZChange.ID, CChange.ID, DChange.ID}))
})
})

Expand All @@ -132,13 +141,43 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, WChange.ID, XChange.ID, YChange.ID, ZChange.ID}))
})

ginkgo.It("should return changes of a root node along with soft", func() {
relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{
CatalogID: U.ID.String(),
Recursive: "downstream",
Soft: true,
})

Expect(err).To(BeNil())

Expect(len(relatedChanges.Changes)).To(Equal(7))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, WChange.ID, XChange.ID, YChange.ID, ZChange.ID, AChange.ID}))
})

ginkgo.It("should return changes of a leaf node", func() {
relatedChanges, err := findChanges(Z.ID, "all", false)
Expect(err).To(BeNil())
Expect(len(relatedChanges.Changes)).To(Equal(4))
Expect(len(relatedChanges.Changes)).To(Equal(5))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{ZChange.ID, XChange.ID, VChange.ID, UChange.ID, DChange.ID}))
})

ginkgo.It("should return changes of a leaf node along with soft", func() {
relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{
CatalogID: X.ID.String(),
Recursive: "downstream",
Soft: true,
})

Expect(err).To(BeNil())

Expect(len(relatedChanges.Changes)).To(Equal(3))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{ZChange.ID, XChange.ID, VChange.ID, UChange.ID}))
Expect(relatedIDs).To(ConsistOf([]string{XChange.ID, ZChange.ID, CChange.ID}))
})
})

Expand All @@ -152,13 +191,43 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID}))
})

ginkgo.It("should return changes of a leaf node along with soft", func() {
relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{
CatalogID: U.ID.String(),
Recursive: "upstream",
Soft: true,
})

Expect(err).To(BeNil())

Expect(len(relatedChanges.Changes)).To(Equal(2))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, AChange.ID}))
})

ginkgo.It("should return changes of a non-root node", func() {
relatedChanges, err := findChanges(X.ID, "all", false)
Expect(err).To(BeNil())
Expect(len(relatedChanges.Changes)).To(Equal(6))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, ZChange.ID, XChange.ID, CChange.ID, DChange.ID}))
})

ginkgo.It("should return changes of a leaf node along with soft", func() {
relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{
CatalogID: X.ID.String(),
Recursive: "upstream",
Soft: true,
})

Expect(err).To(BeNil())

Expect(len(relatedChanges.Changes)).To(Equal(4))

relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID })
Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, ZChange.ID, XChange.ID}))
Expect(relatedIDs).To(ConsistOf([]string{XChange.ID, UChange.ID, VChange.ID, CChange.ID}))
})
})

Expand Down Expand Up @@ -320,10 +389,10 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() {
Recursive: query.CatalogChangeRecursiveAll,
})
Expect(err).To(BeNil())
Expect(len(response.Changes)).To(Equal(5))
Expect(response.Total).To(Equal(int64(5)))
Expect(len(response.Changes)).To(Equal(8))
Expect(response.Total).To(Equal(int64(8)))
Expect(response.Summary["diff"]).To(Equal(3))
Expect(response.Summary["Pulled"]).To(Equal(1))
Expect(response.Summary["Pulled"]).To(Equal(4))
Expect(response.Summary["RegisterNode"]).To(Equal(1))
})
})
Expand Down
43 changes: 41 additions & 2 deletions views/006_config_views.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,8 @@ CREATE OR REPLACE FUNCTION related_changes_recursive (
lookup_id UUID,
type_filter TEXT DEFAULT 'downstream', -- 'downstream', 'upstream', 'all', 'none' or ''
include_deleted_configs BOOLEAN DEFAULT FALSE,
max_depth INTEGER DEFAULT 5
max_depth INTEGER DEFAULT 5,
soft BOOLEAN DEFAULT FALSE
) RETURNS TABLE (
id uuid,
config_id uuid,
Expand Down Expand Up @@ -784,6 +785,45 @@ BEGIN
FROM config_changes cc
LEFT JOIN config_items on config_items.id = cc.config_id
WHERE cc.config_id = lookup_id;

ELSIF type_filter IN ('downstream') THEN
RETURN query
SELECT
cc.id, cc.config_id, config_items.name, config_items.type, config_items.tags, cc.external_created_by,
cc.created_at, cc.severity, cc.change_type, cc.source, cc.summary, cc.created_by, cc.count, cc.first_observed, config_items.agent_id
FROM config_changes cc
LEFT JOIN config_items on config_items.id = cc.config_id
LEFT JOIN
(SELECT config_relationships.config_id, config_relationships.related_id
FROM config_relationships
WHERE relation != 'hard') AS cr
ON (cr.config_id = cc.config_id OR (soft AND cr.related_id = cc.config_id))
WHERE starts_with(config_items.path, (
SELECT CASE
WHEN config_items.path = '' THEN config_items.id::text
ELSE CONCAT(config_items.path, '.', config_items.id)
END
FROM config_items WHERE config_items.id = lookup_id
)) OR
(cc.config_id = lookup_id) OR
(soft AND (cr.config_id = lookup_id OR cr.related_id = lookup_id));

ELSIF type_filter IN ('upstream') THEN
RETURN query
SELECT
cc.id, cc.config_id, config_items.name, config_items.type, config_items.tags, cc.external_created_by,
cc.created_at, cc.severity, cc.change_type, cc.source, cc.summary, cc.created_by, cc.count, cc.first_observed, config_items.agent_id
FROM config_changes cc
LEFT JOIN config_items on config_items.id = cc.config_id
LEFT JOIN
(SELECT config_relationships.config_id, config_relationships.related_id
FROM config_relationships
WHERE relation != 'hard') AS cr
ON (cr.config_id = cc.config_id OR (soft AND cr.related_id = cc.config_id))
WHERE cc.config_id IN (SELECT get_recursive_path.id FROM get_recursive_path(lookup_id)) OR
(cc.config_id = lookup_id) OR
(soft AND (cr.config_id = lookup_id OR cr.related_id = lookup_id));

ELSE
RETURN query
SELECT
Expand All @@ -798,7 +838,6 @@ BEGIN
lookup_id,
CASE
WHEN type_filter = 'upstream' THEN 'incoming'
WHEN type_filter = 'downstream' THEN 'outgoing'
ELSE type_filter
END,
max_depth
Expand Down
Loading