Dont reindex twice (#5001)

* Only reindex a space once at a time

* Add changelog

changelog/unreleased/dont-reindex-twice.md

@@ -0,0 +1,6 @@
+Bugfix: Do not reindex a space twice at the same time
+
+We fixed a problem where the search service reindexed a space while another
+reindex process was still in progress.
+
+https://github.com/owncloud/ocis/pull/5001

services/search/pkg/search/provider/events.go

@@ -19,9 +19,10 @@ import (
 
 // SpaceDebouncer debounces operations on spaces for a configurable amount of time
 type SpaceDebouncer struct {
-	after   time.Duration
-	f       func(id *provider.StorageSpaceId, userID *user.UserId)
-	pending map[string]*time.Timer
+	after      time.Duration
+	f          func(id *provider.StorageSpaceId, userID *user.UserId)
+	pending    map[string]*time.Timer
+	inProgress sync.Map
 
 	mutex sync.Mutex
 }
@@ -29,9 +30,10 @@ type SpaceDebouncer struct {
 // NewSpaceDebouncer returns a new SpaceDebouncer instance
 func NewSpaceDebouncer(d time.Duration, f func(id *provider.StorageSpaceId, userID *user.UserId)) *SpaceDebouncer {
 	return &SpaceDebouncer{
-		after:   d,
-		f:       f,
-		pending: map[string]*time.Timer{},
+		after:      d,
+		f:          f,
+		pending:    map[string]*time.Timer{},
+		inProgress: sync.Map{},
 	}
 }
 
@@ -45,6 +47,16 @@ func (d *SpaceDebouncer) Debounce(id *provider.StorageSpaceId, userID *user.User
 	}
 
 	d.pending[id.OpaqueId] = time.AfterFunc(d.after, func() {
+		if _, ok := d.inProgress.Load(id.OpaqueId); ok {
+			// Reschedule this run for when the previous run has finished
+			d.mutex.Lock()
+			d.pending[id.OpaqueId].Reset(d.after)
+			d.mutex.Unlock()
+			return
+		}
+
+		d.inProgress.Store(id.OpaqueId, true)
+		defer d.inProgress.Delete(id.OpaqueId)
 		d.f(id, userID)
 	})
 }

services/search/pkg/search/provider/events_test.go

@@ -204,6 +204,9 @@ var _ = Describe("SpaceDebouncer", func() {
 		userId = &user.UserId{
 			OpaqueId: "user",
 		}
+		spaceid = &sprovider.StorageSpaceId{
+			OpaqueId: "spaceid",
+		}
 	)
 
 	BeforeEach(func() {
@@ -214,9 +217,6 @@ var _ = Describe("SpaceDebouncer", func() {
 	})
 
 	It("debounces", func() {
-		spaceid := &sprovider.StorageSpaceId{
-			OpaqueId: "spaceid",
-		}
 		debouncer.Debounce(spaceid, userId)
 		debouncer.Debounce(spaceid, userId)
 		debouncer.Debounce(spaceid, userId)
@@ -226,9 +226,6 @@ var _ = Describe("SpaceDebouncer", func() {
 	})
 
 	It("works multiple times", func() {
-		spaceid := &sprovider.StorageSpaceId{
-			OpaqueId: "spaceid",
-		}
 		debouncer.Debounce(spaceid, userId)
 		debouncer.Debounce(spaceid, userId)
 		debouncer.Debounce(spaceid, userId)
@@ -241,4 +238,21 @@ var _ = Describe("SpaceDebouncer", func() {
 			return callCount["spaceid"]
 		}, "200ms").Should(Equal(2))
 	})
+
+	It("doesn't trigger twice simultaneously", func() {
+		debouncer = provider.NewSpaceDebouncer(50*time.Millisecond, func(id *sprovider.StorageSpaceId, _ *user.UserId) {
+			callCount[id.OpaqueId] += 1
+			time.Sleep(300 * time.Millisecond)
+		})
+		debouncer.Debounce(spaceid, userId)
+		time.Sleep(100 * time.Millisecond) // Let it trigger once
+
+		debouncer.Debounce(spaceid, userId)
+		time.Sleep(100 * time.Millisecond) // shouldn't trigger as the other run is still in progress
+		Expect(callCount["spaceid"]).To(Equal(1))
+
+		Eventually(func() int {
+			return callCount["spaceid"]
+		}, "500ms").Should(Equal(2))
+	})
 })