dashboard history: refactor after review

conf/defaults.ini

@@ -188,7 +188,7 @@ snapshot_remove_expired = true
 snapshot_TTL_days = 90
 
 #################################### Dashboards History ##################
-[dashboards.history]
+[dashboards]
 # A setting of 20 (default) means only the last 20 versions will be stored and older versions removed.
 # To keep all dashboard versions you can set this value to 2147483647.
 versions_to_keep = 20

conf/sample.ini

@@ -163,6 +163,7 @@ log_queries =
 # data source proxy whitelist (ip_or_domain:port separated by spaces)
 ;data_source_proxy_whitelist =
 
+#################################### Snapshots ###########################
 [snapshots]
 # snapshot sharing options
 ;external_enabled = true
@@ -175,7 +176,13 @@ log_queries =
 # remove snapshots after 90 days
 ;snapshot_TTL_days = 90
 
-#################################### Users ####################################
+#################################### Dashboards History ##################
+[dashboards]
+# A setting of 20 (default) means only the last 20 versions will be stored and older versions removed.
+# To keep all dashboard versions you can set this value to 2147483647.
+;versions_to_keep = 20
+
+#################################### Users ###############################
 [users]
 # disable user signup / registration
 ;allow_sign_up = true

pkg/services/sqlstore/dashboard_version.go

@@ -3,8 +3,6 @@ package sqlstore
 import (
 	"fmt"
 	"math"
-	"sort"
-	"strconv"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/bus"
@@ -78,9 +76,10 @@ func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 			affectedDashboardsQuery := fmt.Sprintf(`SELECT dashboard_id FROM dashboard_version
 				GROUP BY dashboard_id HAVING COUNT(dashboard_version.id)>%d`, versionsToKeep)
 
-			err := x.Table("dashboard_version").
+			err := sess.Table("dashboard_version").
 				Select("dashboard_version.id, dashboard_version.version, dashboard_version.dashboard_id").
 				Where(fmt.Sprintf("dashboard_id IN (%s)", affectedDashboardsQuery)).
+				Desc("dashboard_version.dashboard_id", "dashboard_version.version").
 				Find(&versions)
 
 			if err != nil {
@@ -89,13 +88,14 @@ func DeleteExpiredVersions(cmd *m.DeleteExpiredVersionsCommand) error {
 
 			// Keep last versionsToKeep versions and delete other
 			versionIdsToDelete := getVersionIDsToDelete(versions, versionsToKeep)
-			versionIdsToDeleteStr := getVersionIDsToDeleteStr(versionIdsToDelete)
-			deleteExpiredSql := fmt.Sprintf("DELETE FROM dashboard_version WHERE id IN (%v)", strings.Join(versionIdsToDeleteStr, ", "))
-			expiredResponse, err := x.Exec(deleteExpiredSql)
-			if err != nil {
-				return err
+			if len(versionIdsToDelete) > 0 {
+				deleteExpiredSql := `DELETE FROM dashboard_version WHERE id IN (?` + strings.Repeat(",?", len(versionIdsToDelete)-1) + `)`
+				expiredResponse, err := sess.Exec(deleteExpiredSql, versionIdsToDelete...)
+				if err != nil {
+					return err
+				}
+				expiredCount, _ = expiredResponse.RowsAffected()
 			}
-			expiredCount, _ = expiredResponse.RowsAffected()
 		}
 
 		sqlog.Debug("Deleted old/expired dashboard versions", "expired", expiredCount)
@@ -110,48 +110,26 @@ type DashboardVersionExp struct {
 	Version     int   `json:"version"`
 }
 
-// Implement sort.Interface for []DashboardVersionExp (sort by Version field)
-type ByVersion []DashboardVersionExp
-
-func (v ByVersion) Len() int {
-	return len(v)
-}
+func getVersionIDsToDelete(versions []DashboardVersionExp, versionsToKeep int) []interface{} {
+	versionIds := make([]interface{}, 0)
 
-func (v ByVersion) Swap(i, j int) {
-	v[i], v[j] = v[j], v[i]
-}
-
-func (v ByVersion) Less(i, j int) bool {
-	return v[i].Version < v[j].Version
-}
+	if len(versions) == 0 {
+		return versionIds
+	}
 
-func getVersionIDsToDelete(versions []DashboardVersionExp, versionsToKeep int) []int64 {
-	dashboards := make(map[int64][]DashboardVersionExp)
+	currentDashboard := versions[0].DashboardId
+	count := 0
 	for _, v := range versions {
-		elem, present := dashboards[v.DashboardId]
-		if present {
-			dashboards[v.DashboardId] = append(elem, v)
+		if v.DashboardId == currentDashboard {
+			count++
 		} else {
-			dashboards[v.DashboardId] = []DashboardVersionExp{v}
+			count = 1
+			currentDashboard = v.DashboardId
 		}
-	}
-
-	versionIds := make([]int64, 0)
-	for dashboard_id, versions := range dashboards {
-		sort.Sort(sort.Reverse(ByVersion(versions)))
-		dashboards[dashboard_id] = versions[versionsToKeep:]
-		for _, ver := range dashboards[dashboard_id] {
-			versionIds = append(versionIds, ver.Id)
+		if count > versionsToKeep {
+			versionIds = append(versionIds, v.Id)
 		}
 	}
 
 	return versionIds
 }
-
-func getVersionIDsToDeleteStr(versionIds []int64) []string {
-	var versionIdsToDeleteStr []string
-	for _, versionId := range versionIds {
-		versionIdsToDeleteStr = append(versionIdsToDeleteStr, strconv.FormatInt(versionId, 10))
-	}
-	return versionIdsToDeleteStr
-}

pkg/setting/setting.go

@@ -523,7 +523,7 @@ func NewConfigContext(args *CommandLineArgs) error {
 
 	// read dashboard settings
 	dashboards := Cfg.Section("dashboards")
-	DashboardVersionsToKeep = dashboards.Key("snapshot_TTL_days").MustInt(20)
+	DashboardVersionsToKeep = dashboards.Key("versions_to_keep").MustInt(20)
 
 	//  read data source proxy white list
 	DataProxyWhiteList = make(map[string]bool)