Database Performance Optimizations #768
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import ( | |
| "time" | ||
|
|
||
| "github.com/grafana/grafana/pkg/services/annotations/accesscontrol" | ||
| "github.com/grafana/grafana/pkg/services/sqlstore/migrator" | ||
|
|
||
| "github.com/grafana/grafana/pkg/infra/db" | ||
| "github.com/grafana/grafana/pkg/infra/log" | ||
|
|
@@ -519,52 +520,135 @@ func (r *xormRepositoryImpl) CleanAnnotations(ctx context.Context, cfg setting.A | |
| var totalAffected int64 | ||
| if cfg.MaxAge > 0 { | ||
| cutoffDate := timeNow().Add(-cfg.MaxAge).UnixNano() / int64(time.Millisecond) | ||
| // Single-statement approaches, specifically ones using batched sub-queries, seem to deadlock with concurrent inserts on MySQL. | ||
| // We have a bounded batch size, so work around this by first loading the IDs into memory and allowing any locks to flush inside each batch. | ||
| // This may under-delete when concurrent inserts happen, but any such annotations will simply be cleaned on the next cycle. | ||
| // | ||
| // We execute the following batched operation repeatedly until either we run out of objects, the context is cancelled, or there is an error. | ||
| affected, err := untilDoneOrCancelled(ctx, func() (int64, error) { | ||
| cond := fmt.Sprintf(`%s AND created < %v ORDER BY id DESC %s`, annotationType, cutoffDate, r.db.GetDialect().Limit(r.cfg.AnnotationCleanupJobBatchSize)) | ||
| ids, err := r.fetchIDs(ctx, "annotation", cond) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| r.log.Error("Annotations to clean by time", "count", len(ids), "ids", ids, "cond", cond, "err", err) | ||
|
|
||
| x, y := r.deleteByIDs(ctx, "annotation", ids) | ||
| r.log.Error("cleaned annotations by time", "count", len(ids), "affected", x, "err", y) | ||
| return x, y | ||
| }) | ||
| totalAffected += affected | ||
| if err != nil { | ||
| return totalAffected, err | ||
| } | ||
| } | ||
|
|
||
| if cfg.MaxCount > 0 { | ||
| // Similar strategy as the above cleanup process, to avoid deadlocks. | ||
| affected, err := untilDoneOrCancelled(ctx, func() (int64, error) { | ||
| cond := fmt.Sprintf(`%s ORDER BY id DESC %s`, annotationType, r.db.GetDialect().LimitOffset(r.cfg.AnnotationCleanupJobBatchSize, cfg.MaxCount)) | ||
| ids, err := r.fetchIDs(ctx, "annotation", cond) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| r.log.Error("Annotations to clean by count", "count", len(ids), "ids", ids, "cond", cond, "err", err) | ||
|
|
||
| x, y := r.deleteByIDs(ctx, "annotation", ids) | ||
| r.log.Error("cleaned annotations by count", "count", len(ids), "affected", x, "err", y) | ||
| return x, y | ||
| }) | ||
| totalAffected += affected | ||
| if err != nil { | ||
| return totalAffected, err | ||
| } | ||
| } | ||
|
|
||
| return totalAffected, nil | ||
| } | ||
|
|
||
| func (r *xormRepositoryImpl) CleanOrphanedAnnotationTags(ctx context.Context) (int64, error) { | ||
| return untilDoneOrCancelled(ctx, func() (int64, error) { | ||
| cond := fmt.Sprintf(`NOT EXISTS (SELECT 1 FROM annotation a WHERE annotation_id = a.id) %s`, r.db.GetDialect().Limit(r.cfg.AnnotationCleanupJobBatchSize)) | ||
| ids, err := r.fetchIDs(ctx, "annotation_tag", cond) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| r.log.Error("Tags to clean", "count", len(ids), "ids", ids, "cond", cond, "err", err) | ||
|
|
||
| x, y := r.deleteByIDs(ctx, "annotation_tag", ids) | ||
| r.log.Error("cleaned tags", "count", len(ids), "affected", x, "err", y) | ||
| return x, y | ||
| }) | ||
| } | ||
|
|
||
| func (r *xormRepositoryImpl) fetchIDs(ctx context.Context, table, condition string) ([]int64, error) { | ||
| sql := fmt.Sprintf(`SELECT id FROM %s`, table) | ||
| if condition == "" { | ||
| return nil, fmt.Errorf("condition must be supplied; cannot fetch IDs from entire table") | ||
| } | ||
| sql += fmt.Sprintf(` WHERE %s`, condition) | ||
| ids := make([]int64, 0) | ||
| err := r.db.WithDbSession(ctx, func(session *db.Session) error { | ||
| return session.SQL(sql).Find(&ids) | ||
| }) | ||
| return ids, err | ||
| } | ||
|
|
||
| func (r *xormRepositoryImpl) deleteByIDs(ctx context.Context, table string, ids []int64) (int64, error) { | ||
| if len(ids) == 0 { | ||
| return 0, nil | ||
| } | ||
|
|
||
| sql := "" | ||
| args := make([]any, 0) | ||
|
|
||
| // SQLite has a parameter limit of 999. | ||
| // If the batch size is bigger than that, and we're on SQLite, we have to put the IDs directly into the statement. | ||
| const sqliteParameterLimit = 999 | ||
| if r.db.GetDBType() == migrator.SQLite && r.cfg.AnnotationCleanupJobBatchSize > sqliteParameterLimit { | ||
| values := fmt.Sprint(ids[0]) | ||
| for _, v := range ids[1:] { | ||
| values = fmt.Sprintf("%s, %d", values, v) | ||
| } | ||
| sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, values) | ||
|
Comment on lines
+609
to
+613
Author
There was a problem hiding this comment.
O(N²) memory allocation bottleneck exists in the SQLite string concatenation loop because var strIDs []string
for _, id := range ids {
strIDs = append(strIDs, strconv.FormatInt(id, 10))
}
sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, strings.Join(strIDs, ","))Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
| } else { | ||
| placeholders := "?" + strings.Repeat(",?", len(ids)-1) | ||
| sql = fmt.Sprintf(`DELETE FROM %s WHERE id IN (%s)`, table, placeholders) | ||
| args = asAny(ids) | ||
| } | ||
|
Comment on lines
+615
to
+618
Author
There was a problem hiding this comment.
Parameter limit violation in PostgreSQL and MySQL occurs because Prompt for LLMTalk to Kody by mentioning @kody Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction. |
||
|
|
||
| var affected int64 | ||
| err := r.db.WithDbSession(ctx, func(session *db.Session) error { | ||
| res, err := session.Exec(append([]any{sql}, args...)...) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| affected, err = res.RowsAffected() | ||
| return err | ||
| }) | ||
| return affected, err | ||
| } | ||
|
|
||
| func asAny(vs []int64) []any { | ||
| r := make([]any, len(vs)) | ||
| for i, v := range vs { | ||
| r[i] = v | ||
| } | ||
| return r | ||
| } | ||
|
|
||
| // untilDoneOrCancelled repeatedly executes batched work until that work is either done (i.e., returns zero affected objects), | ||
| // a batch produces an error, or the provided context is cancelled. | ||
| // The work to be done is given as a callback that returns the number of affected objects for each batch, plus that batch's errors. | ||
| func untilDoneOrCancelled(ctx context.Context, batchWork func() (int64, error)) (int64, error) { | ||
| var totalAffected int64 | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return totalAffected, ctx.Err() | ||
| default: | ||
| affected, err := batchWork() | ||
| totalAffected += affected | ||
| if err != nil { | ||
| return totalAffected, err | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unintentional
r.log.Errorcalls in the cleanup routines, including MaxCount and OrphanedTags blocks, spam production logs with largeidsarrays and cause false alarms. Remove these statements or change the level tor.log.Debugto prevent log bloat.Prompt for LLM
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.