Annotations: Optimize search by tags (#93547)

* Annotations: Optimize search on large number of dashboards

* refactor

* fix batch size

* Return early if no annotations found

* revert go.mod

* return nil in case of error

* Move default limit to the API package

* fix empty access control filter

* Set default limit to 100

* optimize query when number of annotations is less than limit

* Update pkg/services/annotations/annotationsimpl/annotations.go

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>

* remove limit from store since it's set in API

* set default limit in Find method (do not break tests)

* Only add limit to the query if it's set

* use limit trick for all searches without dashboard filter

* set default page if not provided

---------

Co-authored-by: Ieva <ieva.vasiljeva@grafana.com>
This commit is contained in:
Alexander Zobnin 2024-09-23 17:29:29 +02:00 committed by GitHub
parent 5bc7a47ecb
commit 5e713673e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 85 additions and 40 deletions

View File

@ -22,6 +22,8 @@ import (
"github.com/grafana/grafana/pkg/web"
)
const defaultAnnotationsLimit = 100
// swagger:route GET /annotations annotations getAnnotations
//
// Find Annotations.
@ -48,6 +50,9 @@ func (hs *HTTPServer) GetAnnotations(c *contextmodel.ReqContext) response.Respon
MatchAny: c.QueryBool("matchAny"),
SignedInUser: c.SignedInUser,
}
if query.Limit == 0 {
query.Limit = defaultAnnotationsLimit
}
// When dashboard UID present in the request, we ignore dashboard ID
if query.DashboardUID != "" {

View File

@ -39,7 +39,7 @@ func NewAuthService(db db.DB, features featuremgmt.FeatureToggles) *AuthService
}
// Authorize checks if the user has permission to read annotations, then returns a struct containing dashboards and scope types that the user has access to.
func (authz *AuthService) Authorize(ctx context.Context, orgID int64, query *annotations.ItemQuery) (*AccessResources, error) {
func (authz *AuthService) Authorize(ctx context.Context, query *annotations.ItemQuery) (*AccessResources, error) {
user := query.SignedInUser
if user == nil || user.IsNil() {
return nil, ErrReadForbidden.Errorf("missing user")
@ -60,14 +60,14 @@ func (authz *AuthService) Authorize(ctx context.Context, orgID int64, query *ann
var err error
if canAccessDashAnnotations {
if query.AnnotationID != 0 {
annotationDashboardID, err := authz.getAnnotationDashboard(ctx, query, orgID)
annotationDashboardID, err := authz.getAnnotationDashboard(ctx, query)
if err != nil {
return nil, ErrAccessControlInternal.Errorf("failed to fetch annotations: %w", err)
}
query.DashboardID = annotationDashboardID
}
visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, query, orgID)
visibleDashboards, err = authz.dashboardsWithVisibleAnnotations(ctx, query)
if err != nil {
return nil, ErrAccessControlInternal.Errorf("failed to fetch dashboards: %w", err)
}
@ -80,7 +80,7 @@ func (authz *AuthService) Authorize(ctx context.Context, orgID int64, query *ann
}, nil
}
func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query *annotations.ItemQuery, orgID int64) (int64, error) {
func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query *annotations.ItemQuery) (int64, error) {
var items []annotations.Item
params := make([]any, 0)
err := authz.db.WithDbSession(ctx, func(sess *db.Session) error {
@ -92,7 +92,7 @@ func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query *ann
FROM annotation as a
WHERE a.org_id = ? AND a.id = ?
`
params = append(params, orgID, query.AnnotationID)
params = append(params, query.OrgID, query.AnnotationID)
return sess.SQL(sql, params...).Find(&items)
})
@ -106,7 +106,7 @@ func (authz *AuthService) getAnnotationDashboard(ctx context.Context, query *ann
return items[0].DashboardID, nil
}
func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, query *annotations.ItemQuery, orgID int64) (map[string]int64, error) {
func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context, query *annotations.ItemQuery) (map[string]int64, error) {
recursiveQueriesSupported, err := authz.db.RecursiveQueriesAreSupported()
if err != nil {
return nil, err
@ -119,7 +119,7 @@ func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context,
filters := []any{
permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, dashboardaccess.PERMISSION_VIEW, filterType, authz.features, recursiveQueriesSupported),
searchstore.OrgFilter{OrgId: orgID},
searchstore.OrgFilter{OrgId: query.OrgID},
}
if query.DashboardUID != "" {
@ -134,32 +134,25 @@ func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context,
}
sb := &searchstore.Builder{Dialect: authz.db.GetDialect(), Filters: filters, Features: authz.features}
// This is a limit for a batch size, not for the end query result.
var limit int64 = 1000
if query.Page == 0 {
query.Page = 1
}
sql, params := sb.ToSQL(limit, query.Page)
visibleDashboards := make(map[string]int64)
var res []dashboardProjection
var page int64 = 1
var limit int64 = 1000
for {
var res []dashboardProjection
sql, params := sb.ToSQL(limit, page)
err = authz.db.WithDbSession(ctx, func(sess *db.Session) error {
return sess.SQL(sql, params...).Find(&res)
})
if err != nil {
return nil, err
}
err = authz.db.WithDbSession(ctx, func(sess *db.Session) error {
return sess.SQL(sql, params...).Find(&res)
})
if err != nil {
return nil, err
}
for _, p := range res {
visibleDashboards[p.UID] = p.ID
}
// if the result is less than the limit, we have reached the end
if len(res) < int(limit) {
break
}
page++
for _, p := range res {
visibleDashboards[p.UID] = p.ID
}
return visibleDashboards, nil

View File

@ -175,8 +175,8 @@ func TestIntegrationAuthorize(t *testing.T) {
authz := NewAuthService(sql, featuremgmt.WithFeatures(tc.featureToggle))
query := &annotations.ItemQuery{SignedInUser: u}
resources, err := authz.Authorize(context.Background(), 1, query)
query := &annotations.ItemQuery{SignedInUser: u, OrgID: 1}
resources, err := authz.Authorize(context.Background(), query)
require.NoError(t, err)
if tc.expectedResources.Dashboards != nil {

View File

@ -8,6 +8,8 @@ type AccessResources struct {
CanAccessDashAnnotations bool
// CanAccessOrgAnnotations true if the user is allowed to access organization annotations
CanAccessOrgAnnotations bool
// Skip filtering
SkipAccessControlFilter bool
}
type dashboardProjection struct {

View File

@ -72,12 +72,50 @@ func (r *RepositoryImpl) Update(ctx context.Context, item *annotations.Item) err
}
func (r *RepositoryImpl) Find(ctx context.Context, query *annotations.ItemQuery) ([]*annotations.ItemDTO, error) {
resources, err := r.authZ.Authorize(ctx, query.OrgID, query)
if err != nil {
return make([]*annotations.ItemDTO, 0), err
if query.Limit == 0 {
query.Limit = 100
}
return r.reader.Get(ctx, query, resources)
// Search without dashboard UID filter is expensive, so check without access control first
if query.DashboardID == 0 && query.DashboardUID == "" {
// Return early if no annotations found, it's not necessary to perform expensive access control filtering
res, err := r.reader.Get(ctx, query, &accesscontrol.AccessResources{
SkipAccessControlFilter: true,
})
if err != nil || len(res) == 0 {
return []*annotations.ItemDTO{}, err
}
// If number of resources is less than limit, it makes sense to set query limit to this
// value, otherwise query will be iterating over all user's dashboards since original
// query limit is never reached.
query.Limit = int64(len(res))
}
results := make([]*annotations.ItemDTO, 0, query.Limit)
query.Page = 1
// Iterate over available annotations until query limit is reached
// or all available dashboards are checked
for len(results) < int(query.Limit) {
resources, err := r.authZ.Authorize(ctx, query)
if err != nil {
return nil, err
}
res, err := r.reader.Get(ctx, query, resources)
if err != nil {
return nil, err
}
results = append(results, res...)
query.Page++
// All user's dashboards are fetched
if len(resources.Dashboards) < int(query.Limit) {
break
}
}
return results, nil
}
func (r *RepositoryImpl) Delete(ctx context.Context, params *annotations.DeleteParams) error {

View File

@ -351,14 +351,16 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue
if err != nil {
return err
}
sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter))
if query.Limit == 0 {
query.Limit = 100
if acFilter != "" {
sql.WriteString(fmt.Sprintf(" AND (%s)", acFilter))
}
// order of ORDER BY arguments match the order of a sql index for performance
sql.WriteString(" ORDER BY a.org_id, a.epoch_end DESC, a.epoch DESC" + r.db.GetDialect().Limit(query.Limit) + " ) dt on dt.id = annotation.id")
orderBy := " ORDER BY a.org_id, a.epoch_end DESC, a.epoch DESC"
if query.Limit > 0 {
orderBy += r.db.GetDialect().Limit(query.Limit)
}
sql.WriteString(orderBy + " ) dt on dt.id = annotation.id")
if err := sess.SQL(sql.String(), params...).Find(&items); err != nil {
items = nil
@ -372,6 +374,10 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query *annotations.ItemQue
}
func (r *xormRepositoryImpl) getAccessControlFilter(user identity.Requester, accessResources *accesscontrol.AccessResources) (string, error) {
if accessResources.SkipAccessControlFilter {
return "", nil
}
var filters []string
if accessResources.CanAccessOrgAnnotations {

View File

@ -21,6 +21,7 @@ type ItemQuery struct {
SignedInUser identity.Requester
Limit int64 `json:"limit"`
Page int64
}
// TagsQuery is the query for a tags search.