diff --git a/pkg/services/sqlstore/dashboard_version.go b/pkg/services/sqlstore/dashboard_version.go index d91b5727545..1f2850b2021 100644 --- a/pkg/services/sqlstore/dashboard_version.go +++ b/pkg/services/sqlstore/dashboard_version.go @@ -67,10 +67,10 @@ 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 { - const MAX_VERSIONS_TO_DELETE = 100 - versionsToKeep := setting.DashboardVersionsToKeep if versionsToKeep < 1 { versionsToKeep = 1 @@ -80,27 +80,25 @@ func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error { // 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. - versionIdsToDeleteSubquery := `SELECT id + 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 - ?` + ) AS vtd + WHERE dashboard_version.dashboard_id=vtd.dashboard_id + AND version < vtd.min + vtd.count - ?` var versionIdsToDelete []interface{} - err := sess.SQL(versionIdsToDeleteSubquery, versionsToKeep).Find(&versionIdsToDelete) + err := sess.SQL(versionIdsToDeleteQuery, versionsToKeep).Find(&versionIdsToDelete) if err != nil { return err } // Don't delete more than MAX_VERSIONS_TO_DELETE version per time - limit := MAX_VERSIONS_TO_DELETE - if len(versionIdsToDelete) < MAX_VERSIONS_TO_DELETE { - limit = len(versionIdsToDelete) + if len(versionIdsToDelete) > MAX_VERSIONS_TO_DELETE { + versionIdsToDelete = versionIdsToDelete[:MAX_VERSIONS_TO_DELETE] } - versionIdsToDelete = versionIdsToDelete[:limit] if len(versionIdsToDelete) > 0 { deleteExpiredSql := `DELETE FROM dashboard_version WHERE id IN (?` + strings.Repeat(",?", len(versionIdsToDelete)-1) + `)` diff --git a/pkg/services/sqlstore/dashboard_version_test.go b/pkg/services/sqlstore/dashboard_version_test.go index 151dc7c4be2..a6403755d05 100644 --- a/pkg/services/sqlstore/dashboard_version_test.go +++ b/pkg/services/sqlstore/dashboard_version_test.go @@ -141,5 +141,25 @@ func TestDeleteExpiredVersions(t *testing.T) { 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) + }) }) }