db: don't report compaction cancelled errors

Small change to not report `ErrCancelledCompaction` as a background
error. We still log it as an `Infof`, but we do it in Pebble and not
in the event listener.

Author: RaduBerinde

compaction.go

@@ -2312,8 +2312,7 @@ func (d *DB) compact(c *compaction, errChannel chan error) {
 		d.mu.Lock()
 		c.grantHandle.Started()
 		if err := d.compact1(c, errChannel); err != nil {
-			// TODO(peter): count consecutive compaction errors and backoff.
-			d.opts.EventListener.BackgroundError(err)
+			d.handleCompactFailure(err)
 		}
 		if c.isDownload {
 			d.mu.compact.downloadingCount--
@@ -2351,6 +2350,17 @@ func (d *DB) compact(c *compaction, errChannel chan error) {
 	})
 }
 
+func (d *DB) handleCompactFailure(err error) {
+	if errors.Is(err, ErrCancelledCompaction) {
+		// ErrCancelledCompaction is expected during normal operation, so we don't
+		// want to report it as a background error.
+		d.opts.Logger.Infof("%v", err)
+		return
+	}
+	// TODO(peter): count consecutive compaction errors and backoff.
+	d.opts.EventListener.BackgroundError(err)
+}
+
 // cleanupVersionEdit cleans up any on-disk artifacts that were created
 // for the application of a versionEdit that is no longer going to be applied.
 //

event.go

@@ -115,7 +115,10 @@ type CompactionInfo struct {
 	// always ≥ Duration.
 	TotalDuration time.Duration
 	Done          bool
-	Err           error
+	// Err is set only if Done is true. If non-nil, indicates that the compaction
+	// failed. Note that err can be ErrCancelledCompaction, which can happen
+	// during normal operation.
+	Err error
 
 	SingleLevelOverlappingRatio float64
 	MultiLevelOverlappingRatio  float64
@@ -809,15 +812,6 @@ type EventListener struct {
 	PossibleAPIMisuse func(PossibleAPIMisuseInfo)
 }
 
-func backgroundError(logger Logger, err error) {
-	if errors.Is(err, ErrCancelledCompaction) {
-		// ErrCancelledCompaction is not an unexpected error, hence severity INFO.
-		logger.Infof("background error: %s", err)
-	} else {
-		logger.Errorf("background error: %s", err)
-	}
-}
-
 // EnsureDefaults ensures that background error events are logged to the
 // specified logger if a handler for those events hasn't been otherwise
 // specified. Ensure all handlers are non-nil so that we don't have to check
@@ -826,7 +820,7 @@ func (l *EventListener) EnsureDefaults(logger Logger) {
 	if l.BackgroundError == nil {
 		if logger != nil {
 			l.BackgroundError = func(err error) {
-				backgroundError(logger, err)
+				logger.Errorf("background error: %s", err)
 			}
 		} else {
 			l.BackgroundError = func(error) {}
@@ -915,7 +909,7 @@ func MakeLoggingEventListener(logger Logger) EventListener {
 
 	return EventListener{
 		BackgroundError: func(err error) {
-			backgroundError(logger, err)
+			logger.Errorf("background error: %s", err)
 		},
 		DataCorruption: func(info DataCorruptionInfo) {
 			logger.Errorf("%s", info)

event_listener_test.go

@@ -402,17 +402,6 @@ func newCountingMockLogger(t *testing.T) (*mockLogger, *int, *int) {
 	}, &infoCount, &errorCount
 }
 
-func TestMakeLoggingEventListenerBackgroundErrorCancelledCompaction(t *testing.T) {
-	mockLogger, infoCount, errorCount := newCountingMockLogger(t)
-	e := MakeLoggingEventListener(mockLogger)
-
-	e.BackgroundError(ErrCancelledCompaction)
-	require.Equal(t, 1, *infoCount)
-	require.Equal(t, 0, *errorCount)
-
-	testAllCallbacksSetInEventListener(t, e)
-}
-
 func TestMakeLoggingEventListenerBackgroundErrorOtherError(t *testing.T) {
 	mockLogger, infoCount, errorCount := newCountingMockLogger(t)
 	e := MakeLoggingEventListener(mockLogger)

ingest_test.go

@@ -1411,7 +1411,7 @@ type blockedCompaction struct {
 func TestConcurrentExcise(t *testing.T) {
 	var d, d1, d2 *DB
 	var efos map[string]*EventuallyFileOnlySnapshot
-	backgroundErrs := make(chan error, 5)
+	compactionErrs := make(chan error, 5)
 	var compactions map[string]*blockedCompaction
 	defer func() {
 		for _, e := range efos {
@@ -1447,16 +1447,13 @@ func TestConcurrentExcise(t *testing.T) {
 		}
 		efos = make(map[string]*EventuallyFileOnlySnapshot)
 		compactions = make(map[string]*blockedCompaction)
-		backgroundErrs = make(chan error, 5)
+		compactionErrs = make(chan error, 5)
 
 		var el EventListener
 		el.EnsureDefaults(testLogger{t: t})
 		el.FlushBegin = func(info FlushInfo) {
 			// Don't block flushes
 		}
-		el.BackgroundError = func(err error) {
-			backgroundErrs <- err
-		}
 		el.CompactionBegin = func(info CompactionInfo) {
 			if info.Reason == "move" {
 				return
@@ -1468,6 +1465,11 @@ func TestConcurrentExcise(t *testing.T) {
 				blockedJobID = info.JobID
 			}
 		}
+		el.CompactionEnd = func(info CompactionInfo) {
+			if info.Err != nil {
+				compactionErrs <- info.Err
+			}
+		}
 		el.TableCreated = func(info TableCreateInfo) {
 			blockedCompactionsMu.Lock()
 			if info.JobID != blockedJobID {
@@ -1819,8 +1821,8 @@ func TestConcurrentExcise(t *testing.T) {
 				return "spun off in separate goroutine"
 			}
 			return "ok"
-		case "wait-for-background-error":
-			err := <-backgroundErrs
+		case "wait-for-compaction-error":
+			err := <-compactionErrs
 			return err.Error()
 		default:
 			return fmt.Sprintf("unknown command: %s", td.Cmd)

testdata/concurrent_excise

@@ -96,7 +96,7 @@ compact a-z
 ----
 ok
 
-wait-for-background-error
+wait-for-compaction-error
 ----
 pebble: compaction cancelled by a concurrent operation, will retry compaction