From 5196749b14e68bd23d56ba0d3e49bd483d3a7dac Mon Sep 17 00:00:00 2001
From: tamirms <tamirms@gmail.com>
Date: Wed, 3 Mar 2021 22:15:20 +0100
Subject: [PATCH] Fix problem with multiple threads updating cache at the same
 time

---
 services/horizon/internal/health.go      | 62 +++++++++++-------------
 services/horizon/internal/health_test.go |  2 +-
 2 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/services/horizon/internal/health.go b/services/horizon/internal/health.go
index 5c54f39367..111bcd9c32 100644
--- a/services/horizon/internal/health.go
+++ b/services/horizon/internal/health.go
@@ -22,25 +22,19 @@ type healthCache struct {
 	lastUpdate time.Time
 	ttl        time.Duration
 	clock      clock.Clock
-	lock       sync.RWMutex
+	lock       sync.Mutex
 }
 
-func (h *healthCache) get() (healthResponse, bool) {
-	h.lock.RLock()
-	defer h.lock.RUnlock()
+func (h *healthCache) get(runCheck func() healthResponse) healthResponse {
+	h.lock.Lock()
+	defer h.lock.Unlock()
 
 	if h.clock.Now().Sub(h.lastUpdate) > h.ttl {
-		return healthResponse{}, false
+		h.response = runCheck()
+		h.lastUpdate = h.clock.Now()
 	}
-	return h.response, true
-}
 
-func (h *healthCache) set(response healthResponse) {
-	h.lock.Lock()
-	defer h.lock.Unlock()
-
-	h.lastUpdate = h.clock.Now()
-	h.response = response
+	return h.response
 }
 
 func newHealthCache(ttl time.Duration) *healthCache {
@@ -60,27 +54,29 @@ type healthResponse struct {
 	CoreSynced        bool `json:"core_synced"`
 }
 
-func (h healthCheck) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	response, ok := h.cache.get()
-	if !ok {
-		response = healthResponse{
-			DatabaseConnected: true,
-			CoreUp:            true,
-			CoreSynced:        true,
-		}
-		if err := h.session.Ping(); err != nil {
-			log.Warnf("could ping db: %s", err)
-			response.DatabaseConnected = false
-		}
-		if resp, err := h.core.Info(h.ctx); err != nil {
-			log.Warnf("request to stellar core failed: %s", err)
-			response.CoreUp = false
-			response.CoreSynced = false
-		} else {
-			response.CoreSynced = resp.IsSynced()
-		}
-		h.cache.set(response)
+func (h healthCheck) runCheck() healthResponse {
+	response := healthResponse{
+		DatabaseConnected: true,
+		CoreUp:            true,
+		CoreSynced:        true,
 	}
+	if err := h.session.Ping(); err != nil {
+		log.Warnf("could ping db: %s", err)
+		response.DatabaseConnected = false
+	}
+	if resp, err := h.core.Info(h.ctx); err != nil {
+		log.Warnf("request to stellar core failed: %s", err)
+		response.CoreUp = false
+		response.CoreSynced = false
+	} else {
+		response.CoreSynced = resp.IsSynced()
+	}
+
+	return response
+}
+
+func (h healthCheck) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	response := h.cache.get(h.runCheck)
 
 	if !response.DatabaseConnected || !response.CoreSynced || !response.CoreUp {
 		w.WriteHeader(http.StatusServiceUnavailable)
diff --git a/services/horizon/internal/health_test.go b/services/horizon/internal/health_test.go
index 0501e3ad8e..8871da6273 100644
--- a/services/horizon/internal/health_test.go
+++ b/services/horizon/internal/health_test.go
@@ -160,7 +160,7 @@ func TestHealthCheckCache(t *testing.T) {
 			response:   cachedResponse,
 			lastUpdate: time.Unix(0, 0),
 			ttl:        5 * time.Second,
-			lock:       sync.RWMutex{},
+			lock:       sync.Mutex{},
 		},
 	}