stats: imp code, docs

internal/stats/stats.go

@@ -11,9 +11,11 @@ import (
 // the statistics unit.
 type UnitIDGenFunc func() (id uint32)
 
-// DiskConfig - configuration settings that are stored on disk
+// DiskConfig is the configuration structure that is stored in file.
 type DiskConfig struct {
-	Interval uint32 `yaml:"statistics_interval"` // time interval for statistics (in days)
+	// Interval is the number of days for which the statistics are collected
+	// before flushing to the database.
+	Interval uint32 `yaml:"statistics_interval"`
 }
 
 // Config is the configuration structure for the statistics collecting.
@@ -35,7 +37,7 @@ type Config struct {
 
 	// LimitDays is the maximum number of days to collect statistics into the
 	// current unit.
-	LimitDays uint32 // time limit (in days)
+	LimitDays uint32
 }
 
 // Interface is the statistics interface to be used by other packages.
@@ -86,7 +88,7 @@ const (
 type Entry struct {
 	// Clients is the client's primary ID.
 	//
-	// TODO(a.garipov):  Make this a {net.IP, string} enum?
+	// TODO(a.garipov): Make this a {net.IP, string} enum?
 	Client string
 
 	// Domain is the domain name requested.

internal/stats/unit.go

@@ -26,8 +26,8 @@ const (
 	maxClients = 100 // max number of top clients to store in file or return via Get()
 )
 
-// StatsCtx collects the statistics and flushes it to the database.  It's
-// default flushing interval is one hour.
+// StatsCtx collects the statistics and flushes it to the database.  Its default
+// flushing interval is one hour.
 //
 // TODO(e.burkov):  Use atomic.Pointer for accessing curr and db in go1.19.
 type StatsCtx struct {
@@ -86,14 +86,6 @@ type unit struct {
 	clients map[string]uint64
 }
 
-// ID returns the current unit's id.  It's safe for concurrent use.
-func (u *unit) ID() (id uint32) {
-	u.mu.RLock()
-	defer u.mu.RUnlock()
-
-	return u.id
-}
-
 // ongoing returns the current unit.  It's safe for concurrent use.
 //
 // Note that the unit itself should be locked before accessing.
@@ -330,16 +322,17 @@ func newUnit(id uint32) (u *unit) {
 	}
 }
 
-// beginTxn opens a new database transaction.  It returns nil if the transaction
-// can't be created.
-func beginTxn(db *bbolt.DB, wr bool) (tx *bbolt.Tx) {
+// beginTxn opens a new database transaction.  If writable is true, the
+// transaction will be opened for writing, and for reading otherwise.  It
+// returns nil if the transaction can't be created.
+func beginTxn(db *bbolt.DB, writable bool) (tx *bbolt.Tx) {
 	if db == nil {
 		return nil
 	}
 
 	log.Tracef("opening a database transaction")
 
-	tx, err := db.Begin(wr)
+	tx, err := db.Begin(writable)
 	if err != nil {
 		log.Error("stats: opening a transaction: %s", err)
 
@@ -398,7 +391,8 @@ func unitNameToID(name []byte) (id uint32, ok bool) {
 func (s *StatsCtx) periodicFlush() {
 	for ptr := s.ongoing(); ptr != nil; ptr = s.ongoing() {
 		id := s.unitIDGen()
-		if !s.isEnabled() || ptr.ID() == id {
+		// Access the unit's ID with atomic to avoid locking the whole unit.
+		if !s.isEnabled() || atomic.LoadUint32(&ptr.id) == id {
 			time.Sleep(time.Second)
 
 			continue
@@ -416,9 +410,9 @@ func (s *StatsCtx) periodicFlush() {
 			continue
 		}
 
-		ok1 := flushUnitToDB(tx, u.id, udb)
-		ok2 := s.deleteUnit(tx, id-atomic.LoadUint32(&s.limitHours))
-		if ok1 || ok2 {
+		flushOK := flushUnitToDB(tx, u.id, udb)
+		delOK := s.deleteUnit(tx, id-atomic.LoadUint32(&s.limitHours))
+		if flushOK || delOK {
 			s.commitTxn(tx)
 		} else {
 			_ = tx.Rollback()