From 756aad7827ab23b7e3a697693f3cb47b49515059 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Fri, 17 May 2024 18:58:44 -0500 Subject: [PATCH] fix: listener silent fail on replica Update hasql-notifications to include the fix on https://github.com/diogob/hasql-notifications/issues/24. Which now reveals the following error: ``` $ postgrest-with-postgresql-16 --replica -f test/spec/fixtures/load.sql postgrest-run 17/May/2024:18:35:38 -0500: Successfully connected to PostgreSQL 16.2 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 13.2.0, 64-bit 17/May/2024:18:35:38 -0500: Could not listen for notifications on the "pgrst" channel. ERROR: cannot execute LISTEN during recovery 17/May/2024:18:35:38 -0500: Retrying listening for notifications... ``` This is still not good because the LISTEN channel will be retried forever without a backoff. --- CHANGELOG.md | 1 + cabal.project.freeze | 2 +- nix/overlays/haskell-packages.nix | 6 +++--- postgrest.cabal | 2 +- src/PostgREST/AppState.hs | 10 ++++++---- src/PostgREST/Observation.hs | 20 ++++++++++++-------- stack.yaml | 1 + stack.yaml.lock | 7 +++++++ 8 files changed, 32 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 667ccc351..1f5411314 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #3424, Admin `/live` and `/ready` now differentiates a failure as 500 status - @steve-chavez + 503 status is still given when postgREST is in a recovering state - #3478, Media Types are parsed case insensitively - @develop7 + - #2781, Fix listener silently failing on read replica - @steve-chavez ### Deprecated diff --git a/cabal.project.freeze b/cabal.project.freeze index e0a62e960..54331ccce 100644 --- a/cabal.project.freeze +++ b/cabal.project.freeze @@ -1 +1 @@ -index-state: hackage.haskell.org 2024-04-15T20:28:44Z +index-state: hackage.haskell.org 2024-05-17T23:41:49Z diff --git a/nix/overlays/haskell-packages.nix b/nix/overlays/haskell-packages.nix index b7c0f3b96..804f97c52 100644 --- a/nix/overlays/haskell-packages.nix +++ b/nix/overlays/haskell-packages.nix @@ -38,7 +38,7 @@ let # Notes: # - When adding a new package version here, update cabal. # + Update postgrest.cabal with the package version - # + Update cabal.project.freeze. Just set it to the current timestamp then run `cabal build`. It will tell you the correct timestamp for the index state. + # + Update the index-state in cabal.project.freeze. Run `cabal update` which should return the latest index state. # - When adding a new package version here, you have to update stack. # + To update stack.yaml add: # extra-deps: @@ -67,8 +67,8 @@ let hasql-notifications = lib.dontCheck (prev.callHackageDirect { pkg = "hasql-notifications"; - ver = "0.2.1.1"; - sha256 = "sha256-oPhKA/pSQGJvgQyhsi7CVr9iDT7uWpKUz0iJfXsaxXo="; + ver = "0.2.2.0"; + sha256 = "sha256-73OQ9/su2qvO7HavF3xuuNWLXSXyB9reBUQDaHys06I="; } { } ); diff --git a/postgrest.cabal b/postgrest.cabal index 11f328f3c..fd7744f29 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -109,7 +109,7 @@ library , gitrev >= 1.2 && < 1.4 , hasql >= 1.6.1.1 && < 1.7 , hasql-dynamic-statements >= 0.3.1 && < 0.4 - , hasql-notifications >= 0.2.1.1 && < 0.3 + , hasql-notifications >= 0.2.2.0 && < 0.3 , hasql-pool >= 1.0.1 && < 1.1 , hasql-transaction >= 1.0.1 && < 1.1 , heredoc >= 0.2 && < 0.3 diff --git a/src/PostgREST/AppState.hs b/src/PostgREST/AppState.hs index e60ad1f05..a4a21580f 100644 --- a/src/PostgREST/AppState.hs +++ b/src/PostgREST/AppState.hs @@ -542,23 +542,25 @@ listener appState@AppState{stateObserver=observer, stateMainThreadId=mainThreadI dbOrError <- acquire $ toUtf8 (addFallbackAppName prettyVersion configDbUri) case dbOrError of Right db -> do - observer $ DBListenerStart dbChannel SQL.listen db $ SQL.toPgIdentifier dbChannel + observer $ DBListenStart dbChannel SQL.waitForNotifications handleNotification db Left err -> do - observer $ DBListenerFail dbChannel err + observer $ DBListenFail dbChannel (Left err) exitFailure where handleFinally dbChannel False err = do - observer $ DBListenerFailRecoverObs False dbChannel err + observer $ DBListenFail dbChannel (Right err) killThread mainThreadId handleFinally dbChannel True err = do -- if the thread dies, we try to recover - observer $ DBListenerFailRecoverObs True dbChannel err + observer $ DBListenFail dbChannel (Right err) -- assume the pool connection was also lost, call the connection worker connectionWorker appState + -- retry the listener + observer DBListenRetry listener appState conf handleNotification channel msg = diff --git a/src/PostgREST/Observation.hs b/src/PostgREST/Observation.hs index a0078f102..f4632678b 100644 --- a/src/PostgREST/Observation.hs +++ b/src/PostgREST/Observation.hs @@ -40,9 +40,9 @@ data Observation | SchemaCacheLoadedObs Double | ConnectionRetryObs Int | ConnectionPgVersionErrorObs SQL.UsageError - | DBListenerStart Text - | DBListenerFail Text SQL.ConnectionError - | DBListenerFailRecoverObs Bool Text (Either SomeException ()) + | DBListenStart Text + | DBListenFail Text (Either SQL.ConnectionError (Either SomeException ())) + | DBListenRetry | DBListenerGotSCacheMsg ByteString | DBListenerGotConfigMsg ByteString | ConfigReadErrorObs SQL.UsageError @@ -97,12 +97,16 @@ observationMessage = \case "Attempting to reconnect to the database in " <> (show delay::Text) <> " seconds..." ConnectionPgVersionErrorObs usageErr -> jsonMessage usageErr - DBListenerStart channel -> do + DBListenStart channel -> do "Listening for notifications on the " <> show channel <> " channel" - DBListenerFail channel err -> do - "Could not listen for notifications on the " <> channel <> " channel. " <> show err - DBListenerFailRecoverObs recover channel err -> - "Could not listen for notifications on the " <> channel <> " channel. " <> showListenerError err <> (if recover then " Retrying listening for notifications.." else mempty) + DBListenFail channel listenErr -> + "Failed listening for notifications on the " <> show channel <> " channel. " <> ( + case listenErr of + Left err -> show err + Right err -> showListenerError err + ) + DBListenRetry -> + "Retrying listening for notifications..." DBListenerGotSCacheMsg channel -> "Received a schema cache reload message on the " <> show channel <> " channel" DBListenerGotConfigMsg channel -> diff --git a/stack.yaml b/stack.yaml index f93dfd7fc..987599fc1 100644 --- a/stack.yaml +++ b/stack.yaml @@ -11,4 +11,5 @@ nix: extra-deps: - fuzzyset-0.2.4 + - hasql-notifications-0.2.2.0 - hasql-pool-1.0.1 diff --git a/stack.yaml.lock b/stack.yaml.lock index 8c73f05a2..d7e46e30b 100644 --- a/stack.yaml.lock +++ b/stack.yaml.lock @@ -11,6 +11,13 @@ packages: size: 574 original: hackage: fuzzyset-0.2.4 +- completed: + hackage: hasql-notifications-0.2.2.0@sha256:a4e591ef3f06647b056567d3b66948c4a85371f05deb5434edb6ce190f7c845d,2021 + pantry-tree: + sha256: bd7192a5e82ef6dbac711c3433408a0330c8db1cd3482be1ccd4fbd0a63bc2f6 + size: 452 + original: + hackage: hasql-notifications-0.2.2.0 - completed: hackage: hasql-pool-1.0.1@sha256:3cfb4c7153a6c536ac7e126c17723e6d26ee03794954deed2d72bcc826d05a40,2302 pantry-tree: