Repository: owncloud/ocis
Pull request #5001 (2022-11-08 08:17:08)

Dont reindex twice
This PR changes the search service so that it doesn't reindex a space while another reindex process is still in progress.

sonarcloud[bot]: Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=5001)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=5001&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=5001&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=5001&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=5001&resolved=false&types=CODE_SMELL)

[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5001&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5001&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5001&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=5001&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/changelog/unreleased/dont-reindex-twice.md b/changelog/unreleased/dont-reindex-twice.md
new file mode 100644
index 00000000000..501966f23e6
--- /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 f8b63b58bad..0bcf5c1aa19 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)
 	})
 }


Test patch:
diff --git a/services/search/pkg/search/provider/events_test.go b/services/search/pkg/search/provider/events_test.go
index 9e3275acf75..980b664ce19 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))
+	})
 })