diff --git a/changelog/unreleased/dont-reindex-twice.md b/changelog/unreleased/dont-reindex-twice.md new file mode 100644 index 000000000..501966f23 --- /dev/null +++ b/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 diff --git a/services/search/pkg/search/provider/events.go b/services/search/pkg/search/provider/events.go index f8b63b58b..0bcf5c1aa 100644 --- a/services/search/pkg/search/provider/events.go +++ b/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) }) } diff --git a/services/search/pkg/search/provider/events_test.go b/services/search/pkg/search/provider/events_test.go index 9e3275acf..980b664ce 100644 --- a/services/search/pkg/search/provider/events_test.go +++ b/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)) + }) })