Pull request: querylog: fix oldest calc

Updates #4591.

Squashed commit of the following:

commit 70b70c78c85311363535536c7ea12336b21accf8
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Wed May 25 17:35:54 2022 +0300

    querylog: fix oldest calc

CHANGELOG.md

@@ -129,6 +129,7 @@ In this release, the schema version has changed from 12 to 14.
 
 ### Fixed
 
+- Query log occasionally going into an infinite loop ([#4591]).
 - Service startup on boot on systems using SysV-init ([#4480]).
 - Detection of the stopped service status on macOS and Linux ([#4273]).
 - Case-sensitive ClientID ([#4542]).
@@ -157,6 +158,7 @@ In this release, the schema version has changed from 12 to 14.
 [#4503]: https://github.com/AdguardTeam/AdGuardHome/issues/4503
 [#4533]: https://github.com/AdguardTeam/AdGuardHome/issues/4533
 [#4542]: https://github.com/AdguardTeam/AdGuardHome/issues/4542
+[#4591]: https://github.com/AdguardTeam/AdGuardHome/issues/4591
 [#4592]: https://github.com/AdguardTeam/AdGuardHome/issues/4592
 
 [rfc-9250]:     https://datatracker.ietf.org/doc/html/rfc9250

internal/dnsforward/stats.go

@@ -64,9 +64,9 @@ func (s *Server) logQuery(
 		Answer:            pctx.Res,
 		OrigAnswer:        dctx.origResp,
 		Result:            dctx.result,
-		Elapsed:           elapsed,
 		ClientID:          dctx.clientID,
 		ClientIP:          ip,
+		Elapsed:           elapsed,
 		AuthenticatedData: dctx.responseAD,
 	}
 

internal/home/dns.go

@@ -58,6 +58,7 @@ func initDNSServer() (err error) {
 	}
 
 	conf := querylog.Config{
+		Anonymizer:        anonymizer,
 		ConfigModified:    onConfigModified,
 		HTTPRegister:      httpRegister,
 		FindClient:        Context.clients.findMultiple,
@@ -67,7 +68,6 @@ func initDNSServer() (err error) {
 		Enabled:           config.DNS.QueryLogEnabled,
 		FileEnabled:       config.DNS.QueryLogFileEnabled,
 		AnonymizeClientIP: config.DNS.AnonymizeClientIP,
-		Anonymizer:        anonymizer,
 	}
 	Context.queryLog = querylog.New(conf)
 

internal/querylog/http.go

@@ -19,10 +19,10 @@ import (
 )
 
 type qlogConfig struct {
-	Enabled bool `json:"enabled"`
 	// Use float64 here to support fractional numbers and not mess the API
 	// users by changing the units.
 	Interval          float64 `json:"interval"`
+	Enabled           bool    `json:"enabled"`
 	AnonymizeClientIP bool    `json:"anonymize_client_ip"`
 }
 

internal/querylog/qlog.go

@@ -149,7 +149,7 @@ func (l *queryLog) clear() {
 		log.Error("removing log file %q: %s", l.logFile, err)
 	}
 
-	log.Debug("Query log: cleared")
+	log.Debug("querylog: cleared")
 }
 
 func (l *queryLog) Add(params *AddParams) {

internal/querylog/qlog_test.go

@@ -285,8 +285,8 @@ func addEntry(l *queryLog, host string, answerStr, client net.IP) {
 		Answer:     &a,
 		OrigAnswer: &a,
 		Result:     &res,
-		ClientIP:   client,
 		Upstream:   "upstream",
+		ClientIP:   client,
 	}
 
 	l.Add(params)

internal/querylog/querylog.go

@@ -28,8 +28,11 @@ type QueryLog interface {
 	WriteDiskConfig(c *Config)
 }
 
-// Config - configuration object
+// Config is the query log configuration structure.
 type Config struct {
+	// Anonymizer processes the IP addresses to anonymize those if needed.
+	Anonymizer *aghnet.IPMut
+
 	// ConfigModified is called when the configuration is changed, for
 	// example by HTTP requests.
 	ConfigModified func()
@@ -68,9 +71,6 @@ type Config struct {
 	// AnonymizeClientIP tells if the query log should anonymize clients' IP
 	// addresses.
 	AnonymizeClientIP bool
-
-	// Anonymizer processes the IP addresses to anonymize those if needed.
-	Anonymizer *aghnet.IPMut
 }
 
 // AddParams is the parameters for adding an entry.
@@ -91,18 +91,18 @@ type AddParams struct {
 	// Result is the filtering result (optional).
 	Result *filtering.Result
 
-	// Elapsed is the time spent for processing the request.
-	Elapsed time.Duration
-
 	ClientID string
 
-	ClientIP net.IP
-
 	// Upstream is the URL of the upstream DNS server.
 	Upstream string
 
 	ClientProto ClientProto
 
+	ClientIP net.IP
+
+	// Elapsed is the time spent for processing the request.
+	Elapsed time.Duration
+
 	// Cached indicates if the response is served from cache.
 	Cached bool
 

internal/querylog/search.go

@@ -73,7 +73,7 @@ func (l *queryLog) searchMemory(params *searchParams, cache clientCache) (entrie
 
 // search - searches log entries in the query log using specified parameters
 // returns the list of entries found + time of the oldest entry
-func (l *queryLog) search(params *searchParams) ([]*logEntry, time.Time) {
+func (l *queryLog) search(params *searchParams) (entries []*logEntry, oldest time.Time) {
 	now := time.Now()
 
 	if params.limit == 0 {
@@ -88,7 +88,7 @@ func (l *queryLog) search(params *searchParams) ([]*logEntry, time.Time) {
 	totalLimit := params.offset + params.limit
 
 	// now let's get a unified collection
-	entries := append(memoryEntries, fileEntries...)
+	entries = append(memoryEntries, fileEntries...)
 	if len(entries) > totalLimit {
 		// remove extra records
 		entries = entries[:totalLimit]
@@ -111,13 +111,18 @@ func (l *queryLog) search(params *searchParams) ([]*logEntry, time.Time) {
 		}
 	}
 
-	if len(entries) > 0 && len(entries) <= totalLimit {
+	if len(entries) > 0 {
 		// Update oldest after merging in the memory buffer.
 		oldest = entries[len(entries)-1].Time
 	}
 
-	log.Debug("QueryLog: prepared data (%d/%d) older than %s in %s",
+	log.Debug(
-		len(entries), total, params.olderThan, time.Since(now))
+		"querylog: prepared data (%d/%d) older than %s in %s",
+		len(entries),
+		total,
+		params.olderThan,
+		time.Since(now),
+	)
 
 	return entries, oldest
 }
@@ -180,6 +185,8 @@ func (l *queryLog) searchFiles(
 		e, ts, err = l.readNextEntry(r, params, cache)
 		if err != nil {
 			if err == io.EOF {
+				oldestNano = 0
+
 				break
 			}