From 9d9ffba7e295509e6954419b92f098313b93511f Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sun, 23 Oct 2022 19:45:45 -0400 Subject: [PATCH 1/2] Restoring via follower should have same response --- http/service.go | 69 ++++++++++++++++++++++---------------------- http/service_test.go | 16 ++++++++++ 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/http/service.go b/http/service.go index 5d71532c..0d0e6fae 100644 --- a/http/service.go +++ b/http/service.go @@ -693,50 +693,49 @@ func (s *Service) handleLoad(w http.ResponseWriter, r *http.Request) { Data: b, } err := s.store.Load(lr) - if err != nil { - if err == store.ErrNotLeader { - if redirect { - leaderAPIAddr := s.LeaderAPIAddr() - if leaderAPIAddr == "" { - stats.Add(numLeaderNotFound, 1) - http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable) - return - } - - redirect := s.FormRedirect(r, leaderAPIAddr) - http.Redirect(w, r, redirect, http.StatusMovedPermanently) - return - } - - addr, err := s.store.LeaderAddr() - if err != nil { - http.Error(w, fmt.Sprintf("leader address: %s", err.Error()), - http.StatusInternalServerError) - return - } - if addr == "" { + if err != nil && err != store.ErrNotLeader { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } else if err != nil && err == store.ErrNotLeader { + if redirect { + leaderAPIAddr := s.LeaderAPIAddr() + if leaderAPIAddr == "" { stats.Add(numLeaderNotFound, 1) http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable) return } - username, password, ok := r.BasicAuth() - if !ok { - username = "" - } + redirect := s.FormRedirect(r, leaderAPIAddr) + http.Redirect(w, r, redirect, http.StatusMovedPermanently) + return + } - loadErr := s.cluster.Load(lr, addr, makeCredentials(username, password), timeout) - if loadErr != nil && loadErr.Error() == "unauthorized" { - http.Error(w, "remote load not authorized", http.StatusUnauthorized) - return - } - stats.Add(numRemoteLoads, 1) - w.Header().Add(ServedByHTTPHeader, addr) + addr, err := s.store.LeaderAddr() + if err != nil { + http.Error(w, fmt.Sprintf("leader address: %s", err.Error()), + http.StatusInternalServerError) + return + } + if addr == "" { + stats.Add(numLeaderNotFound, 1) + http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable) return } - http.Error(w, err.Error(), http.StatusBadRequest) - return + username, password, ok := r.BasicAuth() + if !ok { + username = "" + } + + loadErr := s.cluster.Load(lr, addr, makeCredentials(username, password), timeout) + if loadErr != nil && loadErr.Error() == "unauthorized" { + http.Error(w, "remote load not authorized", http.StatusUnauthorized) + return + } + stats.Add(numRemoteLoads, 1) + w.Header().Add(ServedByHTTPHeader, addr) + // Allow this if block to exit, so response remains as before request + // forwarding was put in place. } } else { // No JSON structure expected for this API. diff --git a/http/service_test.go b/http/service_test.go index 0cade6d2..a8da3e96 100644 --- a/http/service_test.go +++ b/http/service_test.go @@ -708,6 +708,13 @@ func Test_LoadOK(t *testing.T) { if resp.StatusCode != http.StatusOK { t.Fatalf("failed to get expected StatusOK for load, got %d", resp.StatusCode) } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("failed to read response body: %s", err.Error()) + } + if exp, got := `{"results":[]}`, string(body); exp != got { + t.Fatalf("incorrect response body, exp: %s, got %s", exp, got) + } } func Test_LoadFlagsNoLeader(t *testing.T) { @@ -749,6 +756,7 @@ func Test_LoadFlagsNoLeader(t *testing.T) { if err != nil { t.Fatalf("failed to make load request") } + defer resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Fatalf("failed to get expected StatusOK for load, got %d", resp.StatusCode) } @@ -756,6 +764,14 @@ func Test_LoadFlagsNoLeader(t *testing.T) { if !clusterLoadCalled { t.Fatalf("cluster load was not called") } + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("failed to read response body: %s", err.Error()) + } + if exp, got := `{"results":[]}`, string(body); exp != got { + t.Fatalf("incorrect response body, exp: %s, got %s", exp, got) + } } func Test_RegisterStatus(t *testing.T) { From 45654fb46fa32704a5e74d906537fe518ee17560 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Sun, 23 Oct 2022 19:47:17 -0400 Subject: [PATCH 2/2] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee7ee7fe..883ffb97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 7.9.1 (unreleased) +### Implementation changes and bug fixes +- [PR #1086](https://github.com/rqlite/rqlite/pull/1086): Restoring via follower should have same HTTP response body. + ## 7.9.0 (October 22nd 2022) This release makes it more convenient to load SQLite files directly into rqlite, as any node can now process the request. For this to work however, all nodes in your cluster must be running 7.9.0 (or later). Otherwse 7.9.0 is fully compatible with earlier release, so a rolling upgrade process is an option.