From 7b61759e9f5c32a9440459dd340ec25c98c28804 Mon Sep 17 00:00:00 2001 From: Brad Davidson Date: Mon, 11 Nov 2024 18:57:03 +0000 Subject: [PATCH] Simplify compact loop error handling Rather than restarting the outer loop on unhandled error, simmply break out of the inner loop on any error. We always want to update the compact and target rev, and do post-compact operations - so the only thing we really need to check the error for is logging and metrics. Signed-off-by: Brad Davidson --- pkg/logstructured/sqllog/sql.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/logstructured/sqllog/sql.go b/pkg/logstructured/sqllog/sql.go index afd62fbc..8c2e66f1 100644 --- a/pkg/logstructured/sqllog/sql.go +++ b/pkg/logstructured/sqllog/sql.go @@ -113,7 +113,6 @@ func (s *SQLLog) compactor(interval time.Duration) { targetCompactRev, _ := s.CurrentRevision(s.ctx) logrus.Tracef("COMPACT starting compactRev=%d targetCompactRev=%d", compactRev, targetCompactRev) -outer: for { select { case <-s.ctx.Done(): @@ -127,12 +126,14 @@ outer: // (several hundred ms) just for the database to execute the subquery to select the revisions to delete. var ( + resultLabel string iterCompactRev int64 compactedRev int64 currentRev int64 err error ) + resultLabel = metrics.ResultSuccess iterCompactRev = compactRev compactedRev = compactRev @@ -147,26 +148,25 @@ outer: compactedRev, currentRev, err = s.compact(compactedRev, iterCompactRev) if err != nil { - // ErrCompacted indicates that no further work is necessary - either compactRev changed since the - // last iteration because another client has compacted, or the requested revision has already been compacted. - if err == server.ErrCompacted { - break - } - logrus.Errorf("Compact failed: %v", err) - metrics.CompactTotal.WithLabelValues(metrics.ResultError).Inc() - continue outer + break } } - if err := s.postCompact(); err != nil { - logrus.Errorf("Post-compact operations failed: %v", err) + if perr := s.postCompact(); perr != nil { + logrus.Errorf("Post-compact operations failed: %v", perr) } // Record the final results for the outer loop compactRev = compactedRev targetCompactRev = currentRev - metrics.CompactTotal.WithLabelValues(metrics.ResultSuccess).Inc() + // ErrCompacted indicates that no further work is necessary - either compactRev changed since the + // last iteration because another client has compacted, or the requested revision has already been compacted. + if err != nil && err != server.ErrCompacted { + logrus.Errorf("Compact failed: %v", err) + resultLabel = metrics.ResultError + } + metrics.CompactTotal.WithLabelValues(resultLabel).Inc() } } @@ -206,7 +206,7 @@ func (s *SQLLog) compact(compactRev int64, targetCompactRev int64) (int64, int64 // Don't bother compacting to a revision that has already been compacted if targetCompactRev <= compactRev { - logrus.Infof("COMPACT revision %d has already been compacted", targetCompactRev) + logrus.Tracef("COMPACT revision %d has already been compacted", targetCompactRev) return dbCompactRev, currentRev, server.ErrCompacted }