Avoid index errors for deleted tenants (#2781)

* Catch backend.ErrDoesNotExist from WriteTenantIndex

* Handle tenant index deletion when already deleted

* Capture notfound from azure.Delete

* Capture notfound from gcs.Delete

* Update changelog

* Add note for why we skip ErrDoesNotExist

* Avoid handling the error twice

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [BUGFIX] Fix panic in metrics summary api [#2738](https://github.com/grafana/tempo/pull/2738) (@mdisibio)
 * [BUGFIX] Fix node role auth IDMSv1 [#2760](https://github.com/grafana/tempo/pull/2760) (@coufalja)
 * [BUGFIX] Only search ingester blocks that fall within the request time range. [#2783](https://github.com/grafana/tempo/pull/2783) (@joe-elliott)
+* [BUGFIX] Fix incorrect metrics for index failures [#2781](https://github.com/grafana/tempo/pull/2781) (@zalegrala)
 
 ## v2.2.0 / 2023-07-31
 

tempodb/backend/azure/azure.go

@@ -138,7 +138,7 @@ func (rw *readerWriter) Delete(ctx context.Context, name string, keypath backend
 	}
 
 	if _, err = blobURL.Delete(ctx, blob.DeleteSnapshotsOptionInclude, blob.BlobAccessConditions{}); err != nil {
-		return errors.Wrapf(err, "error deleting blob, name: %s", name)
+		return readError(err)
 	}
 	return nil
 }

tempodb/backend/gcs/gcs.go

@@ -153,8 +153,7 @@ func (rw *readerWriter) CloseAppend(_ context.Context, tracker backend.AppendTra
 }
 
 func (rw *readerWriter) Delete(ctx context.Context, name string, keypath backend.KeyPath, _ bool) error {
-	return rw.bucket.Object(backend.ObjectFileName(keypath, name)).
-		Delete(ctx)
+	return readError(rw.bucket.Object(backend.ObjectFileName(keypath, name)).Delete(ctx))
 }
 
 // List implements backend.Reader

tempodb/backend/mocks.go

@@ -57,6 +57,7 @@ type MockRawWriter struct {
 	appendBuffer      []byte
 	closeAppendCalled bool
 	deleteCalls       map[string]map[string]int
+	err               error
 }
 
 func (m *MockRawWriter) Write(_ context.Context, _ string, _ KeyPath, data io.Reader, size int64, _ bool) error {
@@ -89,7 +90,7 @@ func (m *MockRawWriter) Delete(_ context.Context, name string, keypath KeyPath,
 	}
 
 	m.deleteCalls[name][strings.Join(keypath, "/")]++
-	return nil
+	return m.err
 }
 
 // MockCompactor

tempodb/backend/raw.go

@@ -91,7 +91,11 @@ func (w *writer) CloseAppend(ctx context.Context, tracker AppendTracker) error {
 func (w *writer) WriteTenantIndex(ctx context.Context, tenantID string, meta []*BlockMeta, compactedMeta []*CompactedBlockMeta) error {
 	// If meta and compactedMeta are empty, call delete the tenant index.
 	if len(meta) == 0 && len(compactedMeta) == 0 {
-		return w.w.Delete(ctx, TenantIndexName, KeyPath([]string{tenantID}), false)
+		// Skip returning an error when the object is already deleted.
+		if err := w.w.Delete(ctx, TenantIndexName, KeyPath([]string{tenantID}), false); err != nil && err != ErrDoesNotExist {
+			return err
+		}
+		return nil
 	}
 
 	b := newTenantIndex(meta, compactedMeta)

tempodb/backend/raw_test.go

@@ -59,6 +59,12 @@ func TestWriter(t *testing.T) {
 
 	expectedDeleteMap := map[string]map[string]int{TenantIndexName: {"test": 1}}
 	assert.Equal(t, expectedDeleteMap, w.(*writer).w.(*MockRawWriter).deleteCalls)
+
+	// When a backend returns ErrDoesNotExist, the tenant index should be deleted, but no error should be returned if the tenant index does not exist
+	m = &MockRawWriter{err: ErrDoesNotExist}
+	w = NewWriter(m)
+	err = w.WriteTenantIndex(ctx, "test", nil, nil)
+	assert.NoError(t, err)
 }
 
 func TestReader(t *testing.T) {