From 1e72960140cbc5f932650f2202ec6290a70185c0 Mon Sep 17 00:00:00 2001
From: Ainar Garipov <a.garipov@adguard.com>
Date: Fri, 29 Oct 2021 13:43:08 +0300
Subject: [PATCH] Pull request: querylog: fix rotation

Updates #3781.

Squashed commit of the following:

commit 43e76450b02f7ec54a1b23e5bb037685c2b89bbf
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Oct 29 13:29:34 2021 +0300

    querylog: imp err handling, names

commit b53cfb9c29473e5e0753169e019be5b73d42361c
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Oct 29 13:17:00 2021 +0300

    querylog: fix rotation
---
 internal/querylog/querylog.go     | 12 ++++----
 internal/querylog/querylogfile.go | 51 ++++++++++++++++++++-----------
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/internal/querylog/querylog.go b/internal/querylog/querylog.go
index b043ef71..0ab20a27 100644
--- a/internal/querylog/querylog.go
+++ b/internal/querylog/querylog.go
@@ -46,11 +46,11 @@ type Config struct {
 	// old log file will be renamed, NOT deleted, so the actual log
 	// retention time is twice the interval.  The value must be one of:
 	//
-	//         6 * time.Hour
-	//        24 * time.Hour
-	//    7 * 24 * time.Hour
-	//   30 * 24 * time.Hour
-	//   90 * 24 * time.Hour
+	//    6 * time.Hour
+	//    1 * timeutil.Day
+	//    7 * timeutil.Day
+	//   30 * timeutil.Day
+	//   90 * timeutil.Day
 	//
 	RotationIvl time.Duration
 
@@ -123,7 +123,7 @@ func newQueryLog(conf Config) (l *queryLog) {
 
 	if !checkInterval(conf.RotationIvl) {
 		log.Info(
-			"querylog: warning: unsupported rotation interval %d, setting to 1 day",
+			"querylog: warning: unsupported rotation interval %s, setting to 1 day",
 			conf.RotationIvl,
 		)
 		l.conf.RotationIvl = timeutil.Day
diff --git a/internal/querylog/querylogfile.go b/internal/querylog/querylogfile.go
index dbf2f59f..a1ad0bf5 100644
--- a/internal/querylog/querylogfile.go
+++ b/internal/querylog/querylogfile.go
@@ -3,6 +3,7 @@ package querylog
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"os"
 	"time"
 
@@ -92,15 +93,15 @@ func (l *queryLog) rotate() error {
 	err := os.Rename(from, to)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
+			log.Debug("querylog: no log to rotate")
+
 			return nil
 		}
 
-		log.Error("querylog: failed to rename file: %s", err)
-
-		return err
+		return fmt.Errorf("failed to rename old file: %w", err)
 	}
 
-	log.Debug("querylog: renamed %s -> %s", from, to)
+	log.Debug("querylog: renamed %s into %s", from, to)
 
 	return nil
 }
@@ -135,22 +136,36 @@ func (l *queryLog) readFileFirstTimeValue() (first time.Time, err error) {
 func (l *queryLog) periodicRotate() {
 	defer log.OnPanic("querylog: rotating")
 
-	var err error
-	for {
-		var oldest time.Time
-		oldest, err = l.readFileFirstTimeValue()
+	rotations := time.NewTicker(1 * timeutil.Day)
+	defer rotations.Stop()
+
+	for range rotations.C {
+		oldest, err := l.readFileFirstTimeValue()
+		if err != nil && !errors.Is(err, os.ErrNotExist) {
+			log.Error("querylog: reading oldest record for rotation: %s", err)
+
+			continue
+		}
+
+		rot := oldest.Add(l.conf.RotationIvl)
+		now := time.Now()
+		if rot.After(time.Now()) {
+			log.Debug(
+				"querylog: %s <= %s, not rotating",
+				now.Format(time.RFC3339),
+				rot.Format(time.RFC3339),
+			)
+
+			continue
+		}
+
+		err = l.rotate()
 		if err != nil {
-			log.Debug("%s", err)
+			log.Error("querylog: rotating: %s", err)
+
+			continue
 		}
 
-		if oldest.Add(l.conf.RotationIvl).After(time.Now()) {
-			err = l.rotate()
-			if err != nil {
-				log.Debug("%s", err)
-			}
-		}
-
-		// What?
-		time.Sleep(timeutil.Day)
+		log.Debug("querylog: rotated successfully")
 	}
 }