Skip to content

Commit

Permalink
Simplify compact loop error handling
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
brandond committed Nov 11, 2024
1 parent 5176be0 commit 7b61759
Showing 1 changed file with 13 additions and 13 deletions.
26 changes: 13 additions & 13 deletions pkg/logstructured/sqllog/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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

Expand All @@ -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()
}
}

Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 7b61759

Please sign in to comment.