db: add Errorf to Logger interface

Add Errorf to the Logger interface. This allows us to log errors at the error level in Cockroach. Informs cockroach/cockroach#112730.
cockroachdb · Oct 25, 2023 · 422dce9 · 422dce9
1 parent 3819121
commit 422dce9
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 14 deletions.
diff --git a/compaction_picker.go b/compaction_picker.go
@@ -1812,7 +1812,7 @@ func pickL0(env compactionEnv, opts *Options, vers *version, baseLevel int) (pc
 	// has been shown to not cause a performance regression.
 	lcf, err := vers.L0Sublevels.PickBaseCompaction(1, vers.Levels[baseLevel].Slice())
 	if err != nil {
-		opts.Logger.Infof("error when picking base compaction: %s", err)
+		opts.Logger.Errorf("error when picking base compaction: %s", err)
 		return
 	}
 	if lcf != nil {
@@ -1830,7 +1830,7 @@ func pickL0(env compactionEnv, opts *Options, vers *version, baseLevel int) (pc
 	// counterproductive.
 	lcf, err = vers.L0Sublevels.PickIntraL0Compaction(env.earliestUnflushedSeqNum, minIntraL0Count)
 	if err != nil {
-		opts.Logger.Infof("error when picking intra-L0 compaction: %s", err)
+		opts.Logger.Errorf("error when picking intra-L0 compaction: %s", err)
 		return
 	}
 	if lcf != nil {

diff --git a/db.go b/db.go
@@ -1985,7 +1985,7 @@ func (d *DB) Metrics() *Metrics {
 
 	metrics.LogWriter.FsyncLatency = d.mu.log.metrics.fsyncLatency
 	if err := metrics.LogWriter.Merge(&d.mu.log.metrics.LogWriterMetrics); err != nil {
-		d.opts.Logger.Infof("metrics error: %s", err)
+		d.opts.Logger.Errorf("metrics error: %s", err)
 	}
 	metrics.Flush.WriteThroughput = d.mu.compact.flushWriteThroughput
 	if d.mu.compact.flushing {
@@ -2582,7 +2582,7 @@ func (d *DB) recycleWAL() (newLogNum base.DiskFileNum, prevLogSize uint64) {
 	metrics := d.mu.log.LogWriter.Metrics()
 	d.mu.Lock()
 	if err := d.mu.log.metrics.Merge(metrics); err != nil {
-		d.opts.Logger.Infof("metrics error: %s", err)
+		d.opts.Logger.Errorf("metrics error: %s", err)
 	}
 	d.mu.Unlock()
 

diff --git a/db_test.go b/db_test.go
@@ -1396,6 +1396,7 @@ type testTracer struct {
 }
 
 func (t *testTracer) Infof(format string, args ...interface{})  {}
+func (t *testTracer) Errorf(format string, args ...interface{}) {}
 func (t *testTracer) Fatalf(format string, args ...interface{}) {}
 
 func (t *testTracer) Eventf(ctx context.Context, format string, args ...interface{}) {

diff --git a/error_test.go b/error_test.go
@@ -20,8 +20,8 @@ import (
 
 type panicLogger struct{}
 
-func (l panicLogger) Infof(format string, args ...interface{}) {
-}
+func (l panicLogger) Infof(format string, args ...interface{})  {}
+func (l panicLogger) Errorf(format string, args ...interface{}) {}
 
 func (l panicLogger) Fatalf(format string, args ...interface{}) {
 	panic(errors.Errorf("fatal: "+format, args...))

diff --git a/event.go b/event.go
@@ -562,7 +562,7 @@ func (l *EventListener) EnsureDefaults(logger Logger) {
 	if l.BackgroundError == nil {
 		if logger != nil {
 			l.BackgroundError = func(err error) {
-				logger.Infof("background error: %s", err)
+				logger.Errorf("background error: %s", err)
 			}
 		} else {
 			l.BackgroundError = func(error) {}
@@ -630,7 +630,7 @@ func MakeLoggingEventListener(logger Logger) EventListener {
 
 	return EventListener{
 		BackgroundError: func(err error) {
-			logger.Infof("background error: %s", err)
+			logger.Errorf("background error: %s", err)
 		},
 		CompactionBegin: func(info CompactionInfo) {
 			logger.Infof("%s", info)

diff --git a/event_listener_test.go b/event_listener_test.go
@@ -318,6 +318,11 @@ func (l redactLogger) Infof(format string, args ...interface{}) {
 	l.logger.Infof("%s", redact.Sprintf(format, args...).Redact())
 }
 
+// Errorf implements the Logger.Errorf interface.
+func (l redactLogger) Errorf(format string, args ...interface{}) {
+	l.logger.Errorf("%s", redact.Sprintf(format, args...).Redact())
+}
+
 // Fatalf implements the Logger.Fatalf interface.
 func (l redactLogger) Fatalf(format string, args ...interface{}) {
 	l.logger.Fatalf("%s", redact.Sprintf(format, args...).Redact())

diff --git a/filenames_test.go b/filenames_test.go
@@ -101,6 +101,10 @@ func (l noFatalLogger) Infof(format string, args ...interface{}) {
 	l.t.Logf(format, args...)
 }
 
+func (l noFatalLogger) Errorf(format string, args ...interface{}) {
+	l.t.Logf(format, args...)
+}
+
 func (l noFatalLogger) Fatalf(format string, args ...interface{}) {
 	l.t.Logf(format, args...)
 }
diff --git a/ingest.go b/ingest.go
@@ -555,7 +555,7 @@ func ingestLink(
 		)
 		if err != nil {
 			if err2 := ingestCleanup(objProvider, lr.localMeta[:i]); err2 != nil {
-				opts.Logger.Infof("ingest cleanup failed: %v", err2)
+				opts.Logger.Errorf("ingest cleanup failed: %v", err2)
 			}
 			return err
 		}
@@ -1399,7 +1399,7 @@ func (d *DB) ingest(
 				err = firstError(err, rkeyIter.Close())
 			}
 			if err != nil {
-				d.opts.Logger.Infof("ingest error reading flushable for log %s: %s", m.logNum, err)
+				d.opts.Logger.Errorf("ingest error reading flushable for log %s: %s", m.logNum, err)
 			}
 		}
 
@@ -1506,14 +1506,14 @@ func (d *DB) ingest(
 
 	if err != nil {
 		if err2 := ingestCleanup(d.objProvider, loadResult.localMeta); err2 != nil {
-			d.opts.Logger.Infof("ingest cleanup failed: %v", err2)
+			d.opts.Logger.Errorf("ingest cleanup failed: %v", err2)
 		}
 	} else {
 		// Since we either created a hard link to the ingesting files, or copied
 		// them over, it is safe to remove the originals paths.
 		for _, path := range loadResult.localPaths {
 			if err2 := d.opts.FS.Remove(path); err2 != nil {
-				d.opts.Logger.Infof("ingest failed to remove original file: %s", err2)
+				d.opts.Logger.Errorf("ingest failed to remove original file: %s", err2)
 			}
 		}
 	}

diff --git a/ingest_test.go b/ingest_test.go
@@ -2681,6 +2681,10 @@ func (l testLogger) Infof(format string, args ...interface{}) {
 	l.t.Logf(format, args...)
 }
 
+func (l testLogger) Errorf(format string, args ...interface{}) {
+	l.t.Logf(format, args...)
+}
+
 func (l testLogger) Fatalf(format string, args ...interface{}) {
 	l.t.Fatalf(format, args...)
 }
@@ -3138,6 +3142,11 @@ func (l *fatalCapturingLogger) Infof(fmt string, args ...interface{}) {
 	l.t.Logf(fmt, args...)
 }
 
+// Errorf implements the Logger interface.
+func (l *fatalCapturingLogger) Errorf(fmt string, args ...interface{}) {
+	l.t.Logf(fmt, args...)
+}
+
 // Fatalf implements the Logger interface.
 func (l *fatalCapturingLogger) Fatalf(_ string, args ...interface{}) {
 	l.err = args[0].(error)

diff --git a/internal/base/logger.go b/internal/base/logger.go
@@ -19,6 +19,7 @@ import (
 // Logger defines an interface for writing log messages.
 type Logger interface {
 	Infof(format string, args ...interface{})
+	Errorf(format string, args ...interface{})
 	Fatalf(format string, args ...interface{})
 }
 type defaultLogger struct{}
@@ -33,6 +34,11 @@ func (defaultLogger) Infof(format string, args ...interface{}) {
 	_ = log.Output(2, fmt.Sprintf(format, args...))
 }
 
+// Errorf implements the Logger.Errorf interface.
+func (defaultLogger) Errorf(format string, args ...interface{}) {
+	_ = log.Output(2, fmt.Sprintf(format, args...))
+}
+
 // Fatalf implements the Logger.Fatalf interface.
 func (defaultLogger) Fatalf(format string, args ...interface{}) {
 	_ = log.Output(2, fmt.Sprintf(format, args...))
@@ -75,6 +81,11 @@ func (b *InMemLogger) Infof(format string, args ...interface{}) {
 	}
 }
 
+// Errorf is part of the Logger interface.
+func (b *InMemLogger) Errorf(format string, args ...interface{}) {
+	b.Infof(format, args...)
+}
+
 // Fatalf is part of the Logger interface.
 func (b *InMemLogger) Fatalf(format string, args ...interface{}) {
 	b.Infof(format, args...)
@@ -125,6 +136,9 @@ var _ LoggerAndTracer = NoopLoggerAndTracer{}
 // Infof implements LoggerAndTracer.
 func (l NoopLoggerAndTracer) Infof(format string, args ...interface{}) {}
 
+// Errorf implements LoggerAndTracer.
+func (l NoopLoggerAndTracer) Errorf(format string, args ...interface{}) {}
+
 // Fatalf implements LoggerAndTracer.
 func (l NoopLoggerAndTracer) Fatalf(format string, args ...interface{}) {}
 

diff --git a/metamorphic/history.go b/metamorphic/history.go
@@ -83,6 +83,12 @@ func (h *history) Infof(format string, args ...interface{}) {
 	_ = h.log.Output(2, h.format("// INFO: ", format, args...))
 }
 
+// Errorf implements the pebble.Logger interface. Note that the output is
+// commented.
+func (h *history) Errorf(format string, args ...interface{}) {
+	_ = h.log.Output(2, h.format("// ERROR: ", format, args...))
+}
+
 // Fatalf implements the pebble.Logger interface. Note that the output is
 // commented.
 func (h *history) Fatalf(format string, args ...interface{}) {

diff --git a/objstorage/objstorageprovider/sharedcache/shared_cache.go b/objstorage/objstorageprovider/sharedcache/shared_cache.go
@@ -872,7 +872,7 @@ func (w *writeWorkers) Start(c *Cache, numWorkers int) {
 					if err != nil {
 						c.metrics.writeBackFailures.Add(1)
 						// TODO(radu): throttle logs.
-						c.logger.Infof("writing back to cache after miss failed: %v", err)
+						c.logger.Errorf("writing back to cache after miss failed: %v", err)
 					}
 				}
 			}

diff --git a/table_cache_test.go b/table_cache_test.go
@@ -1211,7 +1211,8 @@ type catchFatalLogger struct {
 
 var _ Logger = (*catchFatalLogger)(nil)
 
-func (tl *catchFatalLogger) Infof(format string, args ...interface{}) {}
+func (tl *catchFatalLogger) Infof(format string, args ...interface{})  {}
+func (tl *catchFatalLogger) Errorf(format string, args ...interface{}) {}
 
 func (tl *catchFatalLogger) Fatalf(format string, args ...interface{}) {
 	tl.fatalMsgs = append(tl.fatalMsgs, fmt.Sprintf(format, args...))