Folders: Reduce DB queries when counting and deleting resources under folders (#81153)

* Add folder store method for fetching all folder descendants

* Modify GetDescendantCounts() to fetch folder descendants at once

* Reduce DB calls when counting library panels under dashboard

* Reduce DB calls when counting dashboards under folder

* Reduce DB calls during folder delete

* Modify folder registry to count/delete entities under multiple folders

* Reduce DB calls when counting

* Reduce DB calls when deleting
This commit is contained in:
Sofia Papagiannaki
2024-01-30 18:26:34 +02:00
committed by GitHub
parent 0139ac205d
commit 89d3b55bec
24 changed files with 335 additions and 411 deletions
+58 -91
View File
@@ -748,72 +748,62 @@ func (s *Service) Delete(ctx context.Context, cmd *folder.DeleteFolderCommand) e
return dashboards.ErrFolderAccessDenied
}
result := []string{cmd.UID}
folders := []string{cmd.UID}
err = s.db.InTransaction(ctx, func(ctx context.Context) error {
subfolders, err := s.nestedFolderDelete(ctx, cmd)
descendants, err := s.nestedFolderDelete(ctx, cmd)
if err != nil {
logger.Error("the delete folder on folder table failed with err: ", "error", err)
return err
}
result = append(result, subfolders...)
folders = append(folders, descendants...)
dashFolders, err := s.dashboardFolderStore.GetFolders(ctx, cmd.OrgID, result)
if err != nil {
return folder.ErrInternal.Errorf("failed to fetch subfolders from dashboard store: %w", err)
}
for _, foldr := range result {
dashFolder, ok := dashFolders[foldr]
if !ok {
return folder.ErrInternal.Errorf("folder does not exist in dashboard store")
}
if cmd.ForceDeleteRules {
if err := s.deleteChildrenInFolder(ctx, dashFolder.OrgID, dashFolder.UID, cmd.SignedInUser); err != nil {
return err
}
} else {
alertRuleSrv, ok := s.registry[entity.StandardKindAlertRule]
if !ok {
return folder.ErrInternal.Errorf("no alert rule service found in registry")
}
alertRulesInFolder, err := alertRuleSrv.CountInFolder(ctx, dashFolder.OrgID, dashFolder.UID, cmd.SignedInUser)
if err != nil {
s.log.Error("failed to count alert rules in folder", "error", err)
return err
}
if alertRulesInFolder > 0 {
return folder.ErrFolderNotEmpty.Errorf("folder contains %d alert rules", alertRulesInFolder)
}
}
if err = s.legacyDelete(ctx, cmd, dashFolder); err != nil {
if cmd.ForceDeleteRules {
if err := s.deleteChildrenInFolder(ctx, cmd.OrgID, folders, cmd.SignedInUser); err != nil {
return err
}
} else {
alertRuleSrv, ok := s.registry[entity.StandardKindAlertRule]
if !ok {
return folder.ErrInternal.Errorf("no alert rule service found in registry")
}
alertRulesInFolder, err := alertRuleSrv.CountInFolders(ctx, cmd.OrgID, folders, cmd.SignedInUser)
if err != nil {
s.log.Error("failed to count alert rules in folder", "error", err)
return err
}
if alertRulesInFolder > 0 {
return folder.ErrFolderNotEmpty.Errorf("folder contains %d alert rules", alertRulesInFolder)
}
}
if err = s.legacyDelete(ctx, cmd, folders); err != nil {
return err
}
return nil
})
return err
}
func (s *Service) deleteChildrenInFolder(ctx context.Context, orgID int64, folderUID string, user identity.Requester) error {
func (s *Service) deleteChildrenInFolder(ctx context.Context, orgID int64, folderUIDs []string, user identity.Requester) error {
for _, v := range s.registry {
if err := v.DeleteInFolder(ctx, orgID, folderUID, user); err != nil {
if err := v.DeleteInFolders(ctx, orgID, folderUIDs, user); err != nil {
return err
}
}
return nil
}
func (s *Service) legacyDelete(ctx context.Context, cmd *folder.DeleteFolderCommand, dashFolder *folder.Folder) error {
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Folder).Inc()
// nolint:staticcheck
deleteCmd := dashboards.DeleteDashboardCommand{OrgID: cmd.OrgID, ID: dashFolder.ID, ForceDeleteFolderRules: cmd.ForceDeleteRules}
func (s *Service) legacyDelete(ctx context.Context, cmd *folder.DeleteFolderCommand, folderUIDs []string) error {
// TODO use bulk delete
for _, folderUID := range folderUIDs {
deleteCmd := dashboards.DeleteDashboardCommand{OrgID: cmd.OrgID, UID: folderUID, ForceDeleteFolderRules: cmd.ForceDeleteRules}
if err := s.dashboardStore.DeleteDashboard(ctx, &deleteCmd); err != nil {
return toFolderError(err)
if err := s.dashboardStore.DeleteDashboard(ctx, &deleteCmd); err != nil {
return toFolderError(err)
}
}
return nil
}
@@ -907,9 +897,9 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
// the folder store and returns the UIDs for all its descendants.
func (s *Service) nestedFolderDelete(ctx context.Context, cmd *folder.DeleteFolderCommand) ([]string, error) {
logger := s.log.FromContext(ctx)
result := []string{}
descendantUIDs := []string{}
if cmd.SignedInUser == nil {
return result, folder.ErrBadRequest.Errorf("missing signed in user")
return descendantUIDs, folder.ErrBadRequest.Errorf("missing signed in user")
}
_, err := s.Get(ctx, &folder.GetFolderQuery{
@@ -918,31 +908,26 @@ func (s *Service) nestedFolderDelete(ctx context.Context, cmd *folder.DeleteFold
SignedInUser: cmd.SignedInUser,
})
if err != nil {
return result, err
return descendantUIDs, err
}
folders, err := s.store.GetChildren(ctx, folder.GetChildrenQuery{UID: cmd.UID, OrgID: cmd.OrgID})
descendants, err := s.store.GetDescendants(ctx, cmd.OrgID, cmd.UID)
if err != nil {
return result, err
}
for _, f := range folders {
result = append(result, f.UID)
logger.Info("deleting subfolder", "org_id", f.OrgID, "uid", f.UID)
subfolders, err := s.nestedFolderDelete(ctx, &folder.DeleteFolderCommand{UID: f.UID, OrgID: f.OrgID, ForceDeleteRules: cmd.ForceDeleteRules, SignedInUser: cmd.SignedInUser})
if err != nil {
logger.Error("failed deleting subfolder", "org_id", f.OrgID, "uid", f.UID, "error", err)
return result, err
}
result = append(result, subfolders...)
logger.Error("failed to get descendant folders", "error", err)
return descendantUIDs, err
}
logger.Info("deleting folder and its contents", "org_id", cmd.OrgID, "uid", cmd.UID)
err = s.store.Delete(ctx, cmd.UID, cmd.OrgID)
for _, f := range descendants {
descendantUIDs = append(descendantUIDs, f.UID)
}
logger.Info("deleting folder and its descendants", "org_id", cmd.OrgID, "uid", cmd.UID)
toDelete := append(descendantUIDs, cmd.UID)
err = s.store.Delete(ctx, toDelete, cmd.OrgID)
if err != nil {
logger.Info("failed deleting folder", "org_id", cmd.OrgID, "uid", cmd.UID, "err", err)
return result, err
return descendantUIDs, err
}
return result, nil
return descendantUIDs, nil
}
func (s *Service) GetDescendantCounts(ctx context.Context, q *folder.GetDescendantCountsQuery) (folder.DescendantCounts, error) {
@@ -950,54 +935,36 @@ func (s *Service) GetDescendantCounts(ctx context.Context, q *folder.GetDescenda
if q.SignedInUser == nil {
return nil, folder.ErrBadRequest.Errorf("missing signed-in user")
}
if *q.UID == "" {
if q.UID == nil || *q.UID == "" {
return nil, folder.ErrBadRequest.Errorf("missing UID")
}
if q.OrgID < 1 {
return nil, folder.ErrBadRequest.Errorf("invalid orgID")
}
result := []string{*q.UID}
folders := []string{*q.UID}
countsMap := make(folder.DescendantCounts, len(s.registry)+1)
if s.features.IsEnabled(ctx, featuremgmt.FlagNestedFolders) {
subfolders, err := s.getNestedFolders(ctx, q.OrgID, *q.UID)
descendantFolders, err := s.store.GetDescendants(ctx, q.OrgID, *q.UID)
if err != nil {
logger.Error("failed to get subfolders", "error", err)
logger.Error("failed to get descendant folders", "error", err)
return nil, err
}
result = append(result, subfolders...)
countsMap[entity.StandardKindFolder] = int64(len(subfolders))
for _, f := range descendantFolders {
folders = append(folders, f.UID)
}
countsMap[entity.StandardKindFolder] = int64(len(descendantFolders))
}
for _, v := range s.registry {
for _, folder := range result {
c, err := v.CountInFolder(ctx, q.OrgID, folder, q.SignedInUser)
if err != nil {
logger.Error("failed to count folder descendants", "error", err)
return nil, err
}
countsMap[v.Kind()] += c
}
}
return countsMap, nil
}
func (s *Service) getNestedFolders(ctx context.Context, orgID int64, uid string) ([]string, error) {
result := []string{}
folders, err := s.store.GetChildren(ctx, folder.GetChildrenQuery{UID: uid, OrgID: orgID})
if err != nil {
return nil, err
}
for _, f := range folders {
result = append(result, f.UID)
subfolders, err := s.getNestedFolders(ctx, f.OrgID, f.UID)
c, err := v.CountInFolders(ctx, q.OrgID, folders, q.SignedInUser)
if err != nil {
logger.Error("failed to count folder descendants", "error", err)
return nil, err
}
result = append(result, subfolders...)
countsMap[v.Kind()] = c
}
return result, nil
return countsMap, nil
}
// buildSaveDashboardCommand is a simplified version on DashboardServiceImpl.buildSaveDashboardCommand
+12 -20
View File
@@ -259,7 +259,6 @@ func TestIntegrationFolderService(t *testing.T) {
t.Run("When deleting folder by uid should not return access denied error", func(t *testing.T) {
f := folder.NewFolder(util.GenerateShortUID(), "")
f.UID = util.GenerateShortUID()
folderStore.On("GetFolders", mock.Anything, orgID, []string{f.UID}).Return(map[string]*folder.Folder{f.UID: f}, nil)
folderStore.On("GetFolderByUID", mock.Anything, orgID, f.UID).Return(f, nil)
var actualCmd *dashboards.DeleteDashboardCommand
@@ -458,7 +457,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
t.Cleanup(func() {
guardian.New = origNewGuardian
for _, ancestor := range ancestors {
err := serviceWithFlagOn.store.Delete(context.Background(), ancestor.UID, orgID)
err := serviceWithFlagOn.store.Delete(context.Background(), []string{ancestor.UID}, orgID)
assert.NoError(t, err)
}
})
@@ -538,7 +537,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
t.Cleanup(func() {
guardian.New = origNewGuardian
for _, ancestor := range ancestors {
err := serviceWithFlagOn.store.Delete(context.Background(), ancestor.UID, orgID)
err := serviceWithFlagOn.store.Delete(context.Background(), []string{ancestor.UID}, orgID)
assert.NoError(t, err)
}
})
@@ -1392,17 +1391,12 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
t.Cleanup(func() {
guardian.New = origNewGuardian
for _, ancestor := range ancestorFoldersWithPermissions {
err := serviceWithFlagOn.store.Delete(context.Background(), ancestor.UID, orgID)
assert.NoError(t, err)
}
})
t.Cleanup(func() {
guardian.New = origNewGuardian
for _, ancestor := range ancestorFoldersWithoutPermissions {
err := serviceWithFlagOn.store.Delete(context.Background(), ancestor.UID, orgID)
assert.NoError(t, err)
toDelete := make([]string, 0, len(ancestorFoldersWithPermissions)+len(ancestorFoldersWithoutPermissions))
for _, ancestor := range append(ancestorFoldersWithPermissions, ancestorFoldersWithoutPermissions...) {
toDelete = append(toDelete, ancestor.UID)
}
err := serviceWithFlagOn.store.Delete(context.Background(), toDelete, orgID)
assert.NoError(t, err)
})
})
@@ -1435,14 +1429,12 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
t.Cleanup(func() {
guardian.New = origNewGuardian
for _, f := range tree1 {
err := serviceWithFlagOn.store.Delete(context.Background(), f.UID, orgID)
assert.NoError(t, err)
}
for _, f := range tree2 {
err := serviceWithFlagOn.store.Delete(context.Background(), f.UID, orgID)
assert.NoError(t, err)
toDelete := make([]string, 0, len(tree1)+len(tree2))
for _, f := range append(tree1, tree2...) {
toDelete = append(toDelete, f.UID)
}
err := serviceWithFlagOn.store.Delete(context.Background(), toDelete, orgID)
assert.NoError(t, err)
})
testCases := []struct {
+72 -3
View File
@@ -83,11 +83,20 @@ func (ss *sqlStore) Create(ctx context.Context, cmd folder.CreateFolderCommand)
return foldr.WithURL(), err
}
func (ss *sqlStore) Delete(ctx context.Context, uid string, orgID int64) error {
func (ss *sqlStore) Delete(ctx context.Context, UIDs []string, orgID int64) error {
if len(UIDs) == 0 {
return nil
}
return ss.db.WithDbSession(ctx, func(sess *db.Session) error {
_, err := sess.Exec("DELETE FROM folder WHERE uid=? AND org_id=?", uid, orgID)
s := fmt.Sprintf("DELETE FROM folder WHERE org_id=? AND uid IN (%s)", strings.Repeat("?, ", len(UIDs)-1)+"?")
sqlArgs := make([]any, 0, len(UIDs)+2)
sqlArgs = append(sqlArgs, s, orgID)
for _, uid := range UIDs {
sqlArgs = append(sqlArgs, uid)
}
_, err := sess.Exec(sqlArgs...)
if err != nil {
return folder.ErrDatabaseError.Errorf("failed to delete folder: %w", err)
return folder.ErrDatabaseError.Errorf("failed to delete folders: %w", err)
}
return nil
})
@@ -331,6 +340,7 @@ func (ss *sqlStore) getParentsMySQL(ctx context.Context, q folder.GetParentsQuer
return util.Reverse(folders), err
}
// TODO use a single query to get the height of a folder
func (ss *sqlStore) GetHeight(ctx context.Context, foldrUID string, orgID int64, parentUID *string) (int, error) {
height := -1
queue := []string{foldrUID}
@@ -460,6 +470,65 @@ func (ss *sqlStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folde
return folders, nil
}
func (ss *sqlStore) GetDescendants(ctx context.Context, orgID int64, ancestor_uid string) ([]*folder.Folder, error) {
var folders []*folder.Folder
recursiveQueriesAreSupported, err := ss.db.RecursiveQueriesAreSupported()
if err != nil {
return nil, err
}
switch recursiveQueriesAreSupported {
case true:
recQuery := `
WITH RECURSIVE RecQry AS (
SELECT * FROM folder WHERE parent_uid = ? AND org_id = ?
UNION ALL SELECT f.* FROM folder f INNER JOIN RecQry r ON f.parent_uid = r.uid and f.org_id = r.org_id
)
SELECT * FROM RecQry;
`
if err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
err := sess.SQL(recQuery, ancestor_uid, orgID).Find(&folders)
if err != nil {
return folder.ErrDatabaseError.Errorf("failed to get folder descendants: %w", err)
}
return nil
}); err != nil {
return nil, err
}
default:
// this is suboptimal because results is full table scan on f0
// but it's the best we can do without recursive CTE
if err := ss.db.WithDbSession(ctx, func(sess *db.Session) error {
s := strings.Builder{}
args := make([]any, 0, 1+folder.MaxNestedFolderDepth)
args = append(args, orgID)
s.WriteString(`SELECT f0.id, f0.org_id, f0.uid, f0.parent_uid, f0.title, f0.description, f0.created, f0.updated`)
s.WriteString(` FROM folder f0`)
s.WriteString(getFullpathJoinsSQL())
s.WriteString(` WHERE f0.org_id=?`)
s.WriteString(` AND (`)
for i := 1; i <= folder.MaxNestedFolderDepth; i++ {
if i > 1 {
s.WriteString(` OR `)
}
s.WriteString(fmt.Sprintf(`f%d.uid=?`, i))
args = append(args, ancestor_uid)
}
s.WriteString(`)`)
return sess.SQL(s.String(), args...).Find(&folders)
}); err != nil {
return nil, err
}
}
// Add URLs
for i, f := range folders {
folders[i] = f.WithURL()
}
return folders, nil
}
func getFullpathSQL(dialect migrator.Dialect) string {
concatCols := make([]string, 0, folder.MaxNestedFolderDepth)
concatCols = append(concatCols, "COALESCE(REPLACE(f0.title, '/', '\\/'), '')")
+10 -10
View File
@@ -64,7 +64,7 @@ func TestIntegrationCreate(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), f.UID, orgID)
err := folderStore.Delete(context.Background(), []string{f.UID}, orgID)
require.NoError(t, err)
})
@@ -102,7 +102,7 @@ func TestIntegrationCreate(t *testing.T) {
assert.NotEmpty(t, parent.URL)
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), parent.UID, orgID)
err := folderStore.Delete(context.Background(), []string{parent.UID}, orgID)
require.NoError(t, err)
})
assertAncestorUIDs(t, folderStore, parent, []string{folder.GeneralFolderUID})
@@ -117,7 +117,7 @@ func TestIntegrationCreate(t *testing.T) {
})
require.NoError(t, err)
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), f.UID, orgID)
err := folderStore.Delete(context.Background(), []string{f.UID}, orgID)
require.NoError(t, err)
})
@@ -165,7 +165,7 @@ func TestIntegrationDelete(t *testing.T) {
t.Cleanup(func() {
for _, uid := range ancestorUIDs[1:] {
err := folderStore.Delete(context.Background(), uid, orgID)
err := folderStore.Delete(context.Background(), []string{uid}, orgID)
require.NoError(t, err)
}
})
@@ -178,7 +178,7 @@ func TestIntegrationDelete(t *testing.T) {
*/
t.Run("deleting a leaf folder should succeed", func(t *testing.T) {
err := folderStore.Delete(context.Background(), ancestorUIDs[len(ancestorUIDs)-1], orgID)
err := folderStore.Delete(context.Background(), []string{ancestorUIDs[len(ancestorUIDs)-1]}, orgID)
require.NoError(t, err)
children, err := folderStore.GetChildren(context.Background(), folder.GetChildrenQuery{
@@ -221,7 +221,7 @@ func TestIntegrationUpdate(t *testing.T) {
require.NoError(t, err)
require.Equal(t, f.ParentUID, parent.UID)
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), f.UID, orgID)
err := folderStore.Delete(context.Background(), []string{f.UID}, orgID)
require.NoError(t, err)
})
@@ -393,7 +393,7 @@ func TestIntegrationGet(t *testing.T) {
})
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), f.UID, orgID)
err := folderStore.Delete(context.Background(), []string{f.UID}, orgID)
require.NoError(t, err)
})
@@ -489,7 +489,7 @@ func TestIntegrationGetParents(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() {
err := folderStore.Delete(context.Background(), f.UID, orgID)
err := folderStore.Delete(context.Background(), []string{f.UID}, orgID)
require.NoError(t, err)
})
@@ -561,7 +561,7 @@ func TestIntegrationGetChildren(t *testing.T) {
t.Cleanup(func() {
for _, uid := range treeLeaves {
err := folderStore.Delete(context.Background(), uid, orgID)
err := folderStore.Delete(context.Background(), []string{uid}, orgID)
require.NoError(t, err)
}
})
@@ -778,7 +778,7 @@ func TestIntegrationGetFolders(t *testing.T) {
t.Cleanup(func() {
for _, uid := range uids {
err := folderStore.Delete(context.Background(), uid, orgID)
err := folderStore.Delete(context.Background(), []string{uid}, orgID)
require.NoError(t, err)
}
})
+4 -2
View File
@@ -23,8 +23,8 @@ type store interface {
// Create creates a folder and returns the newly-created folder.
Create(ctx context.Context, cmd folder.CreateFolderCommand) (*folder.Folder, error)
// Delete deletes a folder from the folder store.
Delete(ctx context.Context, uid string, orgID int64) error
// Delete folders with the specified UIDs and orgID from the folder store.
Delete(ctx context.Context, UIDs []string, orgID int64) error
// Update updates the given folder's UID, Title, and Description (update mode).
// If the NewParentUID field is not nil, it updates also the parent UID (move mode).
@@ -48,4 +48,6 @@ type store interface {
// GetFolders returns folders with given uids
GetFolders(ctx context.Context, q getFoldersQuery) ([]*folder.Folder, error)
// GetDescendants returns all descendants of a folder
GetDescendants(ctx context.Context, orgID int64, anchestor_uid string) ([]*folder.Folder, error)
}
+5 -1
View File
@@ -28,7 +28,7 @@ func (f *fakeStore) Create(ctx context.Context, cmd folder.CreateFolderCommand)
return f.ExpectedFolder, f.ExpectedError
}
func (f *fakeStore) Delete(ctx context.Context, uid string, orgID int64) error {
func (f *fakeStore) Delete(ctx context.Context, UIDs []string, orgID int64) error {
f.DeleteCalled = true
return f.ExpectedError
}
@@ -60,3 +60,7 @@ func (f *fakeStore) GetHeight(ctx context.Context, folderUID string, orgID int64
func (f *fakeStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folder.Folder, error) {
return f.ExpectedFolders, f.ExpectedError
}
func (f *fakeStore) GetDescendants(ctx context.Context, orgID int64, ancestor_uid string) ([]*folder.Folder, error) {
return f.ExpectedFolders, f.ExpectedError
}