Linter & parts of GH comments

userapi/storage/postgres/stats_table.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/lib/pq"
+	"github.com/matrix-org/dendrite/internal"
 
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/userapi/storage/tables"
@@ -172,24 +173,15 @@ func NewPostgresStatsTable(db *sql.DB, serverName gomatrixserverlib.ServerName)
 }
 
 func (s *statsStatements) startTimers() {
-	// initial run
+	var updateStatsFunc func()
-	time.AfterFunc(time.Minute*5, func() {
+	updateStatsFunc = func() {
 		logrus.Infof("Executing UpdateUserDailyVisits")
 		if err := s.updateUserDailyVisits(context.Background(), nil); err != nil {
 			logrus.WithError(err).Error("failed to update daily user visits")
 		}
-	})
+		time.AfterFunc(time.Hour*3, updateStatsFunc)
-	// every x hours
-	ticker := time.NewTicker(time.Hour * 3)
-	for {
-		select {
-		case <-ticker.C:
-			logrus.Infof("Executing UpdateUserDailyVisits")
-			if err := s.updateUserDailyVisits(context.Background(), nil); err != nil {
-				logrus.WithError(err).Error("failed to update daily user visits")
-			}
-		}
 	}
+	time.AfterFunc(time.Minute*5, updateStatsFunc)
 }
 
 func (s *statsStatements) AllUsers(ctx context.Context, txn *sql.Tx) (result int64, err error) {
@@ -218,6 +210,7 @@ func (s *statsStatements) RegisteredUserByType(ctx context.Context, txn *sql.Tx)
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "RegisteredUserByType: failed to close rows")
 
 	var userType string
 	var count int64
@@ -267,6 +260,7 @@ func (s *statsStatements) R30Users(ctx context.Context, txn *sql.Tx) (map[string
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "R30Users: failed to close rows")
 
 	var platform string
 	var count int64
@@ -303,6 +297,7 @@ func (s *statsStatements) R30UsersV2(ctx context.Context, txn *sql.Tx) (map[stri
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "R30UsersV2: failed to close rows")
 
 	var platform string
 	var count int64

userapi/storage/shared/storage.go

@@ -793,4 +793,4 @@ func (d *Database) R30Users(ctx context.Context) (map[string]int64, error) {
 }
 func (d *Database) R30UsersV2(ctx context.Context) (map[string]int64, error) {
 	return d.Stats.R30UsersV2(ctx, nil)
-}
+}

userapi/storage/sqlite3/stats_table.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/matrix-org/dendrite/internal"
 	"github.com/matrix-org/dendrite/internal/sqlutil"
 	"github.com/matrix-org/dendrite/userapi/storage/tables"
 	"github.com/matrix-org/gomatrixserverlib"
@@ -140,7 +141,7 @@ ON CONFLICT (localpart, device_id, timestamp) DO NOTHING
 
 type statsStatements struct {
 	serverName                    gomatrixserverlib.ServerName
-	db *sql.DB
+	db                            *sql.DB
 	lastUpdate                    time.Time
 	countUsersLastSeenAfterStmt   *sql.Stmt
 	countR30UsersStmt             *sql.Stmt
@@ -154,7 +155,7 @@ func NewSQLiteStatsTable(db *sql.DB, serverName gomatrixserverlib.ServerName) (t
 	s := &statsStatements{
 		serverName: serverName,
 		lastUpdate: time.Now(),
-		db: db,
+		db:         db,
 	}
 
 	_, err := db.Exec(userDailyVisitsSchema)
@@ -173,24 +174,15 @@ func NewSQLiteStatsTable(db *sql.DB, serverName gomatrixserverlib.ServerName) (t
 }
 
 func (s *statsStatements) startTimers() {
-	// initial run
+	var updateStatsFunc func()
-	time.AfterFunc(time.Minute*5, func() {
+	updateStatsFunc = func() {
 		logrus.Infof("Executing UpdateUserDailyVisits")
 		if err := s.updateUserDailyVisits(context.Background(), nil); err != nil {
 			logrus.WithError(err).Error("failed to update daily user visits")
 		}
-	})
+		time.AfterFunc(time.Hour*3, updateStatsFunc)
-	// every x hours
-	ticker := time.NewTicker(time.Hour * 3)
-	for {
-		select {
-		case <-ticker.C:
-			logrus.Infof("Executing UpdateUserDailyVisits")
-			if err := s.updateUserDailyVisits(context.Background(), nil); err != nil {
-				logrus.WithError(err).Error("failed to update daily user visits")
-			}
-		}
 	}
+	time.AfterFunc(time.Minute*5, updateStatsFunc)
 }
 
 func (s *statsStatements) AllUsers(ctx context.Context, txn *sql.Tx) (result int64, err error) {
@@ -201,7 +193,7 @@ func (s *statsStatements) AllUsers(ctx context.Context, txn *sql.Tx) (result int
 	}
 	stmt := sqlutil.TxStmt(txn, queryStmt)
 	err = stmt.QueryRowContext(ctx,
-	1, 2, 3, 4,
+		1, 2, 3, 4,
 	).Scan(&result)
 	return
 }
@@ -229,6 +221,7 @@ func (s *statsStatements) RegisteredUserByType(ctx context.Context, txn *sql.Tx)
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "RegisteredUserByType: failed to close rows")
 
 	var userType string
 	var count int64
@@ -279,6 +272,7 @@ func (s *statsStatements) R30Users(ctx context.Context, txn *sql.Tx) (map[string
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "R30Users: failed to close rows")
 
 	var platform string
 	var count int64
@@ -315,6 +309,7 @@ func (s *statsStatements) R30UsersV2(ctx context.Context, txn *sql.Tx) (map[stri
 	if err != nil {
 		return nil, err
 	}
+	defer internal.CloseAndLogIfError(ctx, rows, "R30UsersV2: failed to close rows")
 
 	var platform string
 	var count int64

userapi/userapi.go

@@ -98,4 +98,3 @@ func NewInternalAPI(
 
 	return userAPI
 }
-

userapi/util/phonehomestats.go

@@ -62,18 +62,12 @@ func StartPhoneHomeCollector(startTime time.Time, cfg *config.Dendrite, userDB s
 	}
 
 	// start initial run after 5min
-	time.AfterFunc(time.Minute * 5, func() {
+	time.AfterFunc(time.Minute*5, p.collect)
-		p.collect()
-	})
 
 	// run every 3 hours
 	ticker := time.NewTicker(time.Hour * 3)
-
+	for range ticker.C {
-	for {
+		p.collect()
-		select {
-		case <-ticker.C:
-			p.collect()
-		}
 	}
 }
 
@@ -89,7 +83,7 @@ func (p *phoneHomeStats) collect() {
 	p.stats["go_os"] = runtime.GOOS
 	p.stats["num_cpu"] = runtime.NumCPU()
 	p.stats["num_go_routine"] = runtime.NumGoroutine()
-	p.stats["uptime_seconds"] = math.Floor(time.Now().Sub(p.startTime).Seconds())
+	p.stats["uptime_seconds"] = math.Floor(time.Since(p.startTime).Seconds())
 
 	ctx, cancel := context.WithTimeout(context.TODO(), time.Minute*1)
 	defer cancel()
@@ -118,10 +112,10 @@ func (p *phoneHomeStats) collect() {
 	// database configuration
 	db, err := sqlutil.Open(&p.cfg.UserAPI.AccountDatabase)
 	if err != nil {
-		logrus.WithError(err).Error("unable to connecto to database")
+		logrus.WithError(err).Error("unable to connect to database")
 		return
 	}
-	defer db.Close()
+	defer internal.CloseAndLogIfError(context.Background(), db, "phoneHomeStats.collect(): failed to close database connection")
 
 	dbVersion := "unknown"
 	dbEngine := "unknown"
@@ -129,14 +123,14 @@ func (p *phoneHomeStats) collect() {
 	case p.cfg.UserAPI.AccountDatabase.ConnectionString.IsSQLite():
 		dbEngine = "SQLite"
 		row := db.QueryRow("select sqlite_version();")
-		if err := row.Scan(&dbVersion); err != nil {
+		if err = row.Scan(&dbVersion); err != nil {
 			logrus.WithError(err).Error("unable to query version")
 			return
 		}
 	case p.cfg.UserAPI.AccountDatabase.ConnectionString.IsPostgres():
 		dbEngine = "Postgres"
 		row := db.QueryRow("SHOW server_version;")
-		if err := row.Scan(&dbVersion); err != nil {
+		if err = row.Scan(&dbVersion); err != nil {
 			logrus.WithError(err).Error("unable to query version")
 			return
 		}
@@ -226,4 +220,4 @@ func (p *phoneHomeStats) collect() {
 		logrus.WithError(err).Warn("unable to send phone home stats")
 		return
 	}
-}
+}

userapi/util/stats.go

@@ -18,7 +18,6 @@
 package util
 
 import (
-	"math"
 	"syscall"
 	"time"
 
@@ -40,8 +39,8 @@ func getMemoryStats(p *phoneHomeStats) error {
 	if usedCPUTime == 0 || newData.timestamp == oldUsage.timestamp {
 		p.stats["cpu_average"] = 0
 	} else {
-		p.stats["cpu_average"] = math.Floor(float64(usedCPUTime / (newData.timestamp - oldUsage.timestamp) * 100))
+		p.stats["cpu_average"] = usedCPUTime / (newData.timestamp - oldUsage.timestamp) * 100
 	}
 	p.stats["memory_rss"] = newUsage.Maxrss
 	return nil
-}
+}