Skip to content

Commit

Permalink
Search: Modify query for better performance (grafana#77576)
Browse files Browse the repository at this point in the history 
* Add missing `org_id` in query condition

* Update benchmarks
  • Loading branch information
@papagian
papagian authored Nov 6, 2023
1 parent 20b4ceb commit f999fe3
Show file tree
Hide file tree
Showing 4 changed files with 184 additions and 64 deletions.
64 changes: 56 additions & 8 deletions pkg/api/folder_bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,49 +96,97 @@ func BenchmarkFolderListAndSearch(b *testing.B) {
features *featuremgmt.FeatureManager
}{
{
desc: "get root folders with nested folders feature enabled",
desc: "impl=default nested_folders=on get root folders",
url: "/api/folders",
expectedLen: LEVEL0_FOLDER_NUM,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=on get root folders",
url: "/api/folders",
expectedLen: LEVEL0_FOLDER_NUM,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "get subfolders with nested folders feature enabled",
desc: "impl=default nested_folders=on get subfolders",
url: "/api/folders?parentUid=folder0",
expectedLen: LEVEL1_FOLDER_NUM,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=on get subfolders",
url: "/api/folders?parentUid=folder0",
expectedLen: LEVEL1_FOLDER_NUM,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "list all inherited dashboards with nested folders feature enabled",
desc: "impl=default nested_folders=on list all inherited dashboards",
url: "/api/search?type=dash-db&limit=5000",
expectedLen: withLimit(all),
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=on list all inherited dashboards",
url: "/api/search?type=dash-db&limit=5000",
expectedLen: withLimit(all),
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "search for pattern with nested folders feature enabled",
desc: "impl=default nested_folders=on search for pattern",
url: "/api/search?type=dash-db&query=dashboard_0_0&limit=5000",
expectedLen: withLimit(1 + LEVEL1_DASHBOARD_NUM + LEVEL2_FOLDER_NUM*LEVEL2_DASHBOARD_NUM),
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=on search for pattern",
url: "/api/search?type=dash-db&query=dashboard_0_0&limit=5000",
expectedLen: withLimit(1 + LEVEL1_DASHBOARD_NUM + LEVEL2_FOLDER_NUM*LEVEL2_DASHBOARD_NUM),
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "search for specific dashboard nested folders feature enabled",
desc: "impl=default nested_folders=on search for specific dashboard",
url: "/api/search?type=dash-db&query=dashboard_0_0_0_0",
expectedLen: 1,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=on search for specific dashboard",
url: "/api/search?type=dash-db&query=dashboard_0_0_0_0",
expectedLen: 1,
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "get root folders with nested folders feature disabled",
desc: "impl=default nested_folders=off get root folders",
url: "/api/folders?limit=5000",
expectedLen: withLimit(LEVEL0_FOLDER_NUM),
features: featuremgmt.WithFeatures(),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=off get root folders",
url: "/api/folders?limit=5000",
expectedLen: withLimit(LEVEL0_FOLDER_NUM),
features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "list all dashboards with nested folders feature disabled",
desc: "impl=default nested_folders=off list all dashboards",
url: "/api/search?type=dash-db&limit=5000",
expectedLen: withLimit(LEVEL0_FOLDER_NUM * LEVEL0_DASHBOARD_NUM),
features: featuremgmt.WithFeatures(),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=off list all dashboards",
url: "/api/search?type=dash-db&limit=5000",
expectedLen: withLimit(LEVEL0_FOLDER_NUM * LEVEL0_DASHBOARD_NUM),
features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
},
{
desc: "search specific dashboard with nested folders feature disabled",
desc: "impl=default nested_folders=off search specific dashboard",
url: "/api/search?type=dash-db&query=dashboard_0_0",
expectedLen: 1,
features: featuremgmt.WithFeatures(),
},
{
desc: "impl=permissionsFilterRemoveSubquery nested_folders=off search specific dashboard",
url: "/api/search?type=dash-db&query=dashboard_0_0",
expectedLen: 1,
features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
Expand Down
20 changes: 12 additions & 8 deletions pkg/services/sqlstore/permissions/dashboard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type PermissionsFilter interface {
Where() (string, []any)

buildClauses()
nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string) (string, []any)
nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string, orgID int64) (string, []any)
}

// NewAccessControlDashboardPermissionFilter creates a new AccessControlDashboardPermissionFilter that is configured with specific actions calculated based on the dashboards.PermissionType and query type
Expand Down Expand Up @@ -126,7 +126,8 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
userID, _ = identity.IntIdentifier(namespaceID, identifier)
}

filter, params := accesscontrol.UserRolesFilter(f.user.GetOrgID(), userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
orgID := f.user.GetOrgID()
filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") "
var args []any
builder := strings.Builder{}
Expand Down Expand Up @@ -208,9 +209,10 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs)
builder.WriteString(fmt.Sprintf("WHERE d.uid IN (SELECT uid FROM %s)", recQueryName))
builder.WriteString(fmt.Sprintf("WHERE d.org_id = ? AND d.uid IN (SELECT uid FROM %s)", recQueryName))
args = append(args, orgID)
default:
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.id")
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.id", orgID)
builder.WriteRune('(')
builder.WriteString(nestedFoldersSelectors)
args = append(args, nestedFoldersArgs...)
Expand All @@ -222,7 +224,8 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
default:
builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
if len(permSelectorArgs) > 0 {
builder.WriteString("WHERE d.uid IN ")
builder.WriteString("WHERE d.org_id = ? AND d.uid IN ")
args = append(args, orgID)
builder.WriteString(permSelector.String())
args = append(args, permSelectorArgs...)
} else {
Expand Down Expand Up @@ -287,7 +290,7 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
builder.WriteString("(dashboard.uid IN ")
builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
default:
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.uid")
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.uid", orgID)
builder.WriteRune('(')
builder.WriteString(nestedFoldersSelectors)
builder.WriteRune(')')
Expand Down Expand Up @@ -372,7 +375,7 @@ func actionsToCheck(actions []string, permissions map[string][]string, wildcards
return toCheck
}

func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string) (string, []any) {
func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string, orgID int64) (string, []any) {
wheres := make([]string, 0, folder.MaxNestedFolderDepth+1)
args := make([]any, 0, len(permSelectorArgs)*(folder.MaxNestedFolderDepth+1))

Expand All @@ -387,7 +390,8 @@ func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSele
s := fmt.Sprintf(tmpl, t, prev, onCol, t, prev, t)
joins = append(joins, s)

wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT %s FROM dashboard d %s WHERE %s.uid IN %s)", leftTableCol, rightTableCol, strings.Join(joins, " "), t, permSelector))
wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT %s FROM dashboard d %s WHERE %s.org_id = ? AND %s.uid IN %s)", leftTableCol, rightTableCol, strings.Join(joins, " "), t, t, permSelector))
args = append(args, orgID)
args = append(args, permSelectorArgs...)

prev = t
Expand Down
12 changes: 7 additions & 5 deletions pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
userID, _ = identity.IntIdentifier(namespaceID, identifier)
}

filter, params := accesscontrol.UserRolesFilter(f.user.GetOrgID(), userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
orgID := f.user.GetOrgID()
filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") "
var args []any
builder := strings.Builder{}
Expand Down Expand Up @@ -124,7 +125,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs)
builder.WriteString("(folder.uid IN (SELECT uid FROM " + recQueryName)
default:
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder.uid", "")
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder.uid", "", orgID)
builder.WriteRune('(')
builder.WriteString(nestedFoldersSelectors)
args = append(args, nestedFoldersArgs...)
Expand Down Expand Up @@ -202,7 +203,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
builder.WriteString("(dashboard.uid IN ")
builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
default:
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "")
nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "", orgID)
builder.WriteRune('(')
builder.WriteString(nestedFoldersSelectors)
builder.WriteRune(')')
Expand Down Expand Up @@ -230,7 +231,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
f.where = clause{string: builder.String(), params: args}
}

func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, _ string) (string, []any) {
func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, _ string, orgID int64) (string, []any) {
wheres := make([]string, 0, folder.MaxNestedFolderDepth+1)
args := make([]any, 0, len(permSelectorArgs)*(folder.MaxNestedFolderDepth+1))

Expand All @@ -247,7 +248,8 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSe
s := fmt.Sprintf(tmpl, t, prev, t, prev, t)
joins = append(joins, s)

wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT f1.uid FROM folder f1 %s WHERE %s.uid IN %s)", leftTableCol, strings.Join(joins, " "), t, permSelector))
wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT f1.uid FROM folder f1 %s WHERE %s.org_id = ? AND %s.uid IN %s)", leftTableCol, strings.Join(joins, " "), t, t, permSelector))
args = append(args, orgID)
args = append(args, permSelectorArgs...)

prev = t
Expand Down
Loading

0 comments on commit f999fe3

Please sign in to comment.