From 9b0dc4614c4a7a34656d786441b31459678718ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20=C5=A0tibran=C3=BD?= Date: Fri, 8 Aug 2025 14:59:59 +0200 Subject: [PATCH] Cleanup old indexes in the same goroutine as build of the index. (#109382) * Cleanup old indexes in the same goroutine as build of the index. * Add test. --- pkg/storage/unified/search/bleve.go | 15 ++++++++++---- pkg/storage/unified/search/bleve_test.go | 25 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/pkg/storage/unified/search/bleve.go b/pkg/storage/unified/search/bleve.go index 1e0d19e2e4f..01dcc3cabd9 100644 --- a/pkg/storage/unified/search/bleve.go +++ b/pkg/storage/unified/search/bleve.go @@ -246,7 +246,7 @@ func (b *bleveBackend) BuildIndex( } } - resourceDir := filepath.Join(b.opts.Root, cleanFileSegment(key.Namespace), cleanFileSegment(fmt.Sprintf("%s.%s", key.Resource, key.Group))) + resourceDir := b.getResourceDir(key) var index bleve.Index cachedIndex := b.getCachedIndex(key) @@ -377,13 +377,20 @@ func (b *bleveBackend) BuildIndex( b.indexMetrics.OpenIndexes.WithLabelValues(idx.indexStorage).Inc() } - // Start a background task to cleanup the old index directories. If we have built a new file-based index, - // the new name is ignored. If we have created in-memory index and fileIndexName is empty, all old directories can be removed. - go b.cleanOldIndexes(resourceDir, fileIndexName) + // Clean up the old index directories. If we have built a new file-based index, the new name is ignored. + // If we have created in-memory index and fileIndexName is empty, all old directories can be removed. + // + // We do the cleanup on the same goroutine as the index building. Using background goroutine could + // cleanup new index directory that is being built by new call to BuildIndex. + b.cleanOldIndexes(resourceDir, fileIndexName) return idx, nil } +func (b *bleveBackend) getResourceDir(key resource.NamespacedResource) string { + return filepath.Join(b.opts.Root, cleanFileSegment(key.Namespace), cleanFileSegment(fmt.Sprintf("%s.%s", key.Resource, key.Group))) +} + func cleanFileSegment(input string) string { input = strings.ReplaceAll(input, string(filepath.Separator), "_") input = strings.ReplaceAll(input, "..", "_") diff --git a/pkg/storage/unified/search/bleve_test.go b/pkg/storage/unified/search/bleve_test.go index a391efde3a1..a5acceaecc8 100644 --- a/pkg/storage/unified/search/bleve_test.go +++ b/pkg/storage/unified/search/bleve_test.go @@ -945,6 +945,12 @@ func TestRebuildingIndexClosesPreviousCachedIndex(t *testing.T) { firstIndex, err := backend.BuildIndex(context.Background(), ns, int64(firstSize), 100, nil, "test", indexTestDocs(ns, firstSize)) require.NoError(t, err) + if testCase.firstInMemory { + verifyDirEntriesCount(t, backend.getResourceDir(ns), 0) + } else { + verifyDirEntriesCount(t, backend.getResourceDir(ns), 1) + } + openInMemoryIndexes := 0 secondSize := 100 @@ -955,6 +961,12 @@ func TestRebuildingIndexClosesPreviousCachedIndex(t *testing.T) { secondIndex, err := backend.BuildIndex(context.Background(), ns, int64(secondSize), 100, nil, "test", indexTestDocs(ns, secondSize)) require.NoError(t, err) + if testCase.secondInMemory { + verifyDirEntriesCount(t, backend.getResourceDir(ns), 0) + } else { + verifyDirEntriesCount(t, backend.getResourceDir(ns), 1) + } + // Verify that first and second index are different, and first one is now closed. require.NotEqual(t, firstIndex, secondIndex) @@ -975,6 +987,19 @@ func TestRebuildingIndexClosesPreviousCachedIndex(t *testing.T) { } } +func verifyDirEntriesCount(t *testing.T, dir string, count int) { + ents, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + ents = nil + // This is fine, if dir doesn't exist. + } else { + require.NoError(t, err) + } + } + require.Len(t, ents, count) +} + func indexTestDocs(ns resource.NamespacedResource, docs int) func(index resource.ResourceIndex) (int64, error) { return func(index resource.ResourceIndex) (int64, error) { var items []*resource.BulkIndexItem