From 0bd436f4b0f1196c49d788804310b8ef79970ca5 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 13 Dec 2021 13:55:41 +0300 Subject: [PATCH] Pull request: querylog: fix logic Merge in DNS/adguard-home from fix-querylog-logs to master Squashed commit of the following: commit db6edb86f1f024c85efd3bca3ceb6dfc6ce04edd Merge: d1981696 a3968658 Author: Ainar Garipov Date: Mon Dec 13 13:48:25 2021 +0300 Merge branch 'master' into fix-querylog-logs commit d1981696782ca9adc5213f76cdbe2dc9f859f921 Author: Ainar Garipov Date: Fri Dec 10 21:14:04 2021 +0300 querylog: fix logic --- internal/aghnet/interfaces.go | 2 ++ internal/querylog/qlogfile.go | 4 +-- internal/querylog/qlogfile_test.go | 6 ++-- internal/querylog/qlogreader.go | 51 ++++++++++++++++------------ internal/querylog/qlogreader_test.go | 4 +-- internal/querylog/search.go | 2 +- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/internal/aghnet/interfaces.go b/internal/aghnet/interfaces.go index ce7f217c..c8efc57b 100644 --- a/internal/aghnet/interfaces.go +++ b/internal/aghnet/interfaces.go @@ -92,6 +92,8 @@ func IfaceDNSIPAddrs( time.Sleep(backoff) } + n-- + switch len(addrs) { case 0: // Don't return errors in case the users want to try and enable diff --git a/internal/querylog/qlogfile.go b/internal/querylog/qlogfile.go index f0c7b7c4..fd6c5226 100644 --- a/internal/querylog/qlogfile.go +++ b/internal/querylog/qlogfile.go @@ -54,7 +54,7 @@ func NewQLogFile(path string) (*QLogFile, error) { }, nil } -// SeekTS performs binary search in the query log file looking for a record +// seekTS performs binary search in the query log file looking for a record // with the specified timestamp. Once the record is found, it sets // "position" so that the next ReadNext call returned that record. // @@ -71,7 +71,7 @@ func NewQLogFile(path string) (*QLogFile, error) { // so that when we call "ReadNext" this line was returned. // * Depth of the search (how many times we compared timestamps). // * If we could not find it, it returns one of the errors described above. -func (q *QLogFile) SeekTS(timestamp int64) (int64, int, error) { +func (q *QLogFile) seekTS(timestamp int64) (int64, int, error) { q.lock.Lock() defer q.lock.Unlock() diff --git a/internal/querylog/qlogfile_test.go b/internal/querylog/qlogfile_test.go index b85eb049..ff1a53b3 100644 --- a/internal/querylog/qlogfile_test.go +++ b/internal/querylog/qlogfile_test.go @@ -175,7 +175,7 @@ func TestQLogFile_SeekTS_good(t *testing.T) { assert.NotEqualValues(t, 0, ts) // Try seeking to that line now. - pos, _, err := q.SeekTS(ts) + pos, _, err := q.seekTS(ts) require.NoError(t, err) assert.NotEqualValues(t, 0, pos) @@ -228,7 +228,7 @@ func TestQLogFile_SeekTS_bad(t *testing.T) { assert.NotEqualValues(t, 0, tc.ts) var depth int - _, depth, err = q.SeekTS(tc.ts) + _, depth, err = q.seekTS(tc.ts) assert.NotEmpty(t, l.num) require.Error(t, err) @@ -340,7 +340,7 @@ func TestQLog_Seek(t *testing.T) { q := NewTestQLogFileData(t, data) - _, depth, err := q.SeekTS(timestamp.Add(time.Second * time.Duration(tc.delta)).UnixNano()) + _, depth, err := q.seekTS(timestamp.Add(time.Second * time.Duration(tc.delta)).UnixNano()) require.Truef(t, errors.Is(err, tc.wantErr), "%v", err) assert.Equal(t, tc.wantDepth, depth) }) diff --git a/internal/querylog/qlogreader.go b/internal/querylog/qlogreader.go index 7735eb67..704dc5ca 100644 --- a/internal/querylog/qlogreader.go +++ b/internal/querylog/qlogreader.go @@ -53,35 +53,44 @@ func NewQLogReader(files []string) (*QLogReader, error) { }, nil } -// SeekTS performs binary search of a query log record with the specified +// seekTS performs binary search of a query log record with the specified // timestamp. If the record is found, it sets QLogReader's position to point to // that line, so that the next ReadNext call returned this line. -func (r *QLogReader) SeekTS(timestamp int64) (err error) { +func (r *QLogReader) seekTS(timestamp int64) (err error) { for i := len(r.qFiles) - 1; i >= 0; i-- { q := r.qFiles[i] - _, _, err = q.SeekTS(timestamp) - if err == nil { - // Search is finished, and the searched element have - // been found. Update currentFile only, position is - // already set properly in QLogFile. - r.currentFile = i + _, _, err = q.seekTS(timestamp) + if err != nil { + if errors.Is(err, ErrTSTooEarly) { + // Look at the next file, since we've reached the end of this + // one. If there is no next file, it's not found. + err = ErrTSNotFound - return nil - } else if errors.Is(err, ErrTSTooEarly) { - // Look at the next file, since we've reached the end of - // this one. - continue - } else if errors.Is(err, ErrTSTooLate) { - // Just seek to the start then. timestamp is probably - // between the end of the previous one and the start of - // this one. - return r.SeekStart() - } else if errors.Is(err, ErrTSNotFound) { - break + continue + } else if errors.Is(err, ErrTSTooLate) { + // Just seek to the start then. timestamp is probably between + // the end of the previous one and the start of this one. + return r.SeekStart() + } else if errors.Is(err, ErrTSNotFound) { + return err + } else { + return fmt.Errorf("seekts: file at index %d: %w", i, err) + } } + + // The search is finished, and the searched element has been found. + // Update currentFile only, position is already set properly in + // QLogFile. + r.currentFile = i + + return nil } - return fmt.Errorf("querylog: %w", err) + if err != nil { + return fmt.Errorf("seekts: %w", err) + } + + return nil } // SeekStart changes the current position to the end of the newest file diff --git a/internal/querylog/qlogreader_test.go b/internal/querylog/qlogreader_test.go index 564508f7..ffdc285b 100644 --- a/internal/querylog/qlogreader_test.go +++ b/internal/querylog/qlogreader_test.go @@ -97,7 +97,7 @@ func TestQLogReader_Seek(t *testing.T) { }, { name: "non-existent_long_ago", time: "2000-02-19T01:23:16.920973+03:00", - want: ErrTSTooEarly, + want: ErrTSNotFound, }, { name: "non-existent_far_ahead", time: "2100-02-19T01:23:16.920973+03:00", @@ -113,7 +113,7 @@ func TestQLogReader_Seek(t *testing.T) { ts, err := time.Parse(time.RFC3339Nano, tc.time) require.NoError(t, err) - err = r.SeekTS(ts.UnixNano()) + err = r.seekTS(ts.UnixNano()) assert.ErrorIs(t, err, tc.want) }) } diff --git a/internal/querylog/search.go b/internal/querylog/search.go index 2181ab04..d387c938 100644 --- a/internal/querylog/search.go +++ b/internal/querylog/search.go @@ -152,7 +152,7 @@ func (l *queryLog) searchFiles( if params.olderThan.IsZero() { err = r.SeekStart() } else { - err = r.SeekTS(params.olderThan.UnixNano()) + err = r.seekTS(params.olderThan.UnixNano()) if err == nil { // Read to the next record, because we only need the one // that goes after it.