diff --git a/pkg/services/sqlstore/dashboard_version.go b/pkg/services/sqlstore/dashboard_version.go index 547f62628f3..1f2850b2021 100644 --- a/pkg/services/sqlstore/dashboard_version.go +++ b/pkg/services/sqlstore/dashboard_version.go @@ -67,30 +67,39 @@ func GetDashboardVersions(query *m.GetDashboardVersionsQuery) error { return nil } +const MAX_VERSIONS_TO_DELETE = 100 + func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error { return inTransaction(func(sess *DBSession) error { - versions := []DashboardVersionExp{} versionsToKeep := setting.DashboardVersionsToKeep - if versionsToKeep < 1 { versionsToKeep = 1 } - err := sess.Table("dashboard_version"). - Select("dashboard_version.id, dashboard_version.version, dashboard_version.dashboard_id"). - Where(`dashboard_id IN ( - SELECT dashboard_id FROM dashboard_version - GROUP BY dashboard_id HAVING COUNT(dashboard_version.id) > ? - )`, versionsToKeep). - Desc("dashboard_version.dashboard_id", "dashboard_version.version"). - Find(&versions) + // Idea of this query is finding version IDs to delete based on formula: + // min_version_to_keep = min_version + (versions_count - versions_to_keep) + // where version stats is processed for each dashboard. This guarantees that we keep at least versions_to_keep + // versions, but in some cases (when versions are sparse) this number may be more. + versionIdsToDeleteQuery := `SELECT id + FROM dashboard_version, ( + SELECT dashboard_id, count(version) as count, min(version) as min + FROM dashboard_version + GROUP BY dashboard_id + ) AS vtd + WHERE dashboard_version.dashboard_id=vtd.dashboard_id + AND version < vtd.min + vtd.count - ?` + var versionIdsToDelete []interface{} + err := sess.SQL(versionIdsToDeleteQuery, versionsToKeep).Find(&versionIdsToDelete) if err != nil { return err } - // Keep last versionsToKeep versions and delete other - versionIdsToDelete := getVersionIDsToDelete(versions, versionsToKeep) + // Don't delete more than MAX_VERSIONS_TO_DELETE version per time + if len(versionIdsToDelete) > MAX_VERSIONS_TO_DELETE { + versionIdsToDelete = versionIdsToDelete[:MAX_VERSIONS_TO_DELETE] + } + if len(versionIdsToDelete) > 0 { deleteExpiredSql := `DELETE FROM dashboard_version WHERE id IN (?` + strings.Repeat(",?", len(versionIdsToDelete)-1) + `)` expiredResponse, err := sess.Exec(deleteExpiredSql, versionIdsToDelete...) @@ -103,34 +112,3 @@ func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error { return nil }) } - -// Short version of DashboardVersion for getting expired versions -type DashboardVersionExp struct { - Id int64 `json:"id"` - DashboardId int64 `json:"dashboardId"` - Version int `json:"version"` -} - -func getVersionIDsToDelete(versions []DashboardVersionExp, versionsToKeep int) []interface{} { - versionIds := make([]interface{}, 0) - - if len(versions) == 0 { - return versionIds - } - - currentDashboard := versions[0].DashboardId - count := 0 - for _, v := range versions { - if v.DashboardId == currentDashboard { - count++ - } else { - count = 1 - currentDashboard = v.DashboardId - } - if count > versionsToKeep { - versionIds = append(versionIds, v.Id) - } - } - - return versionIds -} diff --git a/pkg/services/sqlstore/dashboard_version_test.go b/pkg/services/sqlstore/dashboard_version_test.go index 1b74e7847c4..a6403755d05 100644 --- a/pkg/services/sqlstore/dashboard_version_test.go +++ b/pkg/services/sqlstore/dashboard_version_test.go @@ -136,10 +136,30 @@ func TestDeleteExpiredVersions(t *testing.T) { err := DeleteExpiredVersions(&m.DeleteExpiredVersionsCommand{}) So(err, ShouldBeNil) - query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1} + query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1, Limit: versionsToWrite} GetDashboardVersions(&query) So(len(query.Result), ShouldEqual, versionsToWrite) }) + + Convey("Don't delete more than MAX_VERSIONS_TO_DELETE per iteration", func() { + versionsToWriteBigNumber := MAX_VERSIONS_TO_DELETE + versionsToWrite + for i := 0; i < versionsToWriteBigNumber-versionsToWrite; i++ { + updateTestDashboard(savedDash, map[string]interface{}{ + "tags": "different-tag", + }) + } + + err := DeleteExpiredVersions(&m.DeleteExpiredVersionsCommand{}) + So(err, ShouldBeNil) + + query := m.GetDashboardVersionsQuery{DashboardId: savedDash.Id, OrgId: 1, Limit: versionsToWriteBigNumber} + GetDashboardVersions(&query) + + // Ensure we have at least versionsToKeep versions + So(len(query.Result), ShouldBeGreaterThanOrEqualTo, versionsToKeep) + // Ensure we haven't deleted more than MAX_VERSIONS_TO_DELETE rows + So(versionsToWriteBigNumber-len(query.Result), ShouldBeLessThanOrEqualTo, MAX_VERSIONS_TO_DELETE) + }) }) }