From 8081e038adb848b2762c1bc2392a11d1f8daaa8d Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Tue, 28 Dec 2021 18:45:51 -0500 Subject: [PATCH 1/2] Consider schema cache state on the ready check * empty schema cache when it fails loading so the ready check can pick its new state --- CHANGELOG.md | 2 +- src/PostgREST/Admin.hs | 14 +++++++------- src/PostgREST/AppState.hs | 5 ++--- src/PostgREST/Workers.hs | 10 ++++++---- test/Main.hs | 4 ++-- test/io-tests/db_config.sql | 8 ++++++++ test/io-tests/test_io.py | 26 ++++++++++++++++++++++++++ 7 files changed, 52 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04716db458..60d8b03103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #1933, #2109, Add a minimal health check endpoint - @steve-chavez + For enabling this, the `admin-server-port` config must be set explictly - + The check is at the `:/live` endpoint. A 200 OK status will be returned if postgrest is alive, otherwise a 503 will be returned. + + A `:/live` endpoint is available for checking if postgrest is running on its port/socket. 200 OK = alive, 503 = dead. + A `:/ready` endpoint is available for checking a correct internal state(the database connection plus the schema cache). 200 OK = ready, 503 = not ready. ### Fixed diff --git a/src/PostgREST/Admin.hs b/src/PostgREST/Admin.hs index a10f2a35c4..2f29229059 100644 --- a/src/PostgREST/Admin.hs +++ b/src/PostgREST/Admin.hs @@ -21,16 +21,16 @@ import Protolude -- | PostgREST admin application postgrestAdmin :: AppState.AppState -> AppConfig -> Wai.Application postgrestAdmin appState appConfig req respond = do - isMainAppReachable <- isRight <$> reachMainApp appConfig + isMainAppReachable <- isRight <$> reachMainApp appConfig + isSchemaCacheLoaded <- isJust <$> AppState.getDbStructure appState + isConnectionUp <- + if configDbChannelEnabled appConfig + then AppState.getIsListenerOn appState + else isRight <$> SQL.use (AppState.getPool appState) (SQL.sql "SELECT 1") case Wai.pathInfo req of ["ready"] -> - if configDbChannelEnabled appConfig then do - listenerOn <- AppState.getIsListenerOn appState - respond $ Wai.responseLBS (if listenerOn && isMainAppReachable then HTTP.status200 else HTTP.status503) [] mempty - else do - result <- SQL.use (AppState.getPool appState) $ SQL.sql "SELECT 1" - respond $ Wai.responseLBS (if isRight result && isMainAppReachable then HTTP.status200 else HTTP.status503) [] mempty + respond $ Wai.responseLBS (if isMainAppReachable && isConnectionUp && isSchemaCacheLoaded then HTTP.status200 else HTTP.status503) [] mempty ["live"] -> respond $ Wai.responseLBS (if isMainAppReachable then HTTP.status200 else HTTP.status503) [] mempty _ -> diff --git a/src/PostgREST/AppState.hs b/src/PostgREST/AppState.hs index d6163b9d7b..c4da56fe8c 100644 --- a/src/PostgREST/AppState.hs +++ b/src/PostgREST/AppState.hs @@ -108,9 +108,8 @@ putPgVersion = atomicWriteIORef . statePgVersion getDbStructure :: AppState -> IO (Maybe DbStructure) getDbStructure = readIORef . stateDbStructure -putDbStructure :: AppState -> DbStructure -> IO () -putDbStructure appState structure = - atomicWriteIORef (stateDbStructure appState) $ Just structure +putDbStructure :: AppState -> Maybe DbStructure -> IO () +putDbStructure appState = atomicWriteIORef (stateDbStructure appState) getJsonDbS :: AppState -> IO ByteString getJsonDbS = readIORef . stateJsonDbS diff --git a/src/PostgREST/Workers.hs b/src/PostgREST/Workers.hs index 5fdd3bca77..5cdc5fd05a 100644 --- a/src/PostgREST/Workers.hs +++ b/src/PostgREST/Workers.hs @@ -91,6 +91,7 @@ connectionWorker appState = do -- do nothing and proceed if the load was successful return () SCOnRetry -> + -- retry reloading the schema cache work SCFatalFail -> -- die if our schema cache query has an error @@ -169,12 +170,13 @@ loadSchemaCache appState = do AppState.logWithZTime appState hint return SCFatalFail Nothing -> do + AppState.putDbStructure appState Nothing AppState.logWithZTime appState "An error ocurred when loading the schema cache" putErr return SCOnRetry Right dbStructure -> do - AppState.putDbStructure appState dbStructure + AppState.putDbStructure appState (Just dbStructure) when (isJust configDbRootSpec) . AppState.putJsonDbS appState . LBS.toStrict $ JSON.encode dbStructure AppState.logWithZTime appState "Schema cache loaded" @@ -247,7 +249,7 @@ reReadConfig startingUp appState = do AppState.logWithZTime appState hint killThread (AppState.getMainThreadId appState) Nothing -> do - AppState.logWithZTime appState $ show e + putErr pure [] Right x -> pure x else @@ -257,10 +259,10 @@ reReadConfig startingUp appState = do if startingUp then panic err -- die on invalid config if the program is starting up else - AppState.logWithZTime appState $ "Failed re-loading config: " <> err + AppState.logWithZTime appState $ "Failed reloading config: " <> err Right newConf -> do AppState.putConfig appState newConf if startingUp then pass else - AppState.logWithZTime appState "Config re-loaded" + AppState.logWithZTime appState "Config reloaded" diff --git a/test/Main.hs b/test/Main.hs index a292efd6a2..4202e0040c 100644 --- a/test/Main.hs +++ b/test/Main.hs @@ -76,7 +76,7 @@ main = do let config = cfg testDbConn appState <- AppState.initWithPool pool config AppState.putPgVersion appState actualPgVersion - AppState.putDbStructure appState baseDbStructure + AppState.putDbStructure appState (Just baseDbStructure) when (isJust $ configDbRootSpec config) $ AppState.putJsonDbS appState $ toS $ JSON.encode baseDbStructure return ((), postgrest LogCrit appState $ pure ()) @@ -91,7 +91,7 @@ main = do actualPgVersion appState <- AppState.initWithPool pool config AppState.putPgVersion appState actualPgVersion - AppState.putDbStructure appState customDbStructure + AppState.putDbStructure appState (Just customDbStructure) when (isJust $ configDbRootSpec config) $ AppState.putJsonDbS appState $ toS $ JSON.encode baseDbStructure return ((), postgrest LogCrit appState $ pure ()) diff --git a/test/io-tests/db_config.sql b/test/io-tests/db_config.sql index c0fee5a826..7f2dffd2aa 100644 --- a/test/io-tests/db_config.sql +++ b/test/io-tests/db_config.sql @@ -53,3 +53,11 @@ ALTER ROLE other_authenticator SET pgrst.db_pre_request = 'test.other_custom_hea ALTER ROLE other_authenticator SET pgrst.db_max_rows = '100'; ALTER ROLE other_authenticator SET pgrst.db_extra_search_path = 'public, extensions, other'; ALTER ROLE other_authenticator SET pgrst.openapi_mode = 'disabled'; + +-- limited authenticator used for failed schema cache loads +CREATE ROLE limited_authenticator LOGIN NOINHERIT; + +create or replace function no_schema_cache_for_limited_authenticator() returns void as $_$ +begin + ALTER ROLE limited_authenticator SET statement_timeout to 1; +end $_$ volatile security definer language plpgsql ; diff --git a/test/io-tests/test_io.py b/test/io-tests/test_io.py index 5c5e788807..9a16380f97 100644 --- a/test/io-tests/test_io.py +++ b/test/io-tests/test_io.py @@ -768,6 +768,32 @@ def test_admin_ready_wo_channel(defaultenv): assert response.status_code == 200 +def test_admin_ready_includes_schema_cache_state(defaultenv): + "Should get a failed response from the admin server ready endpoint when the schema cache is not loaded" + + db_uri = defaultenv["PGRST_DB_URI"].replace( + "postgrest_test_authenticator", "limited_authenticator" + ) + env = { + **defaultenv, + "PGRST_DB_URI": db_uri, + "PGRST_DB_ANON_ROLE": "limited_authenticator", + } + + with run(env=env, adminport=freeport()) as postgrest: + + # make it impossible to load the schema cache + response = postgrest.session.post( + "/rpc/no_schema_cache_for_limited_authenticator" + ) + assert response.status_code == 200 + # force a reconnection so the new role setting is picked up + postgrest.process.send_signal(signal.SIGUSR1) + time.sleep(0.1) + response = postgrest.admin.get("/ready") + assert response.status_code == 503 + + def test_admin_not_found(defaultenv): "Should get a not found from a undefined endpoint on the admin server" From 35a750728ef19e1318ad31d0142c52209cc61581 Mon Sep 17 00:00:00 2001 From: steve-chavez Date: Tue, 28 Dec 2021 20:43:30 -0500 Subject: [PATCH 2/2] fix: clarify error for failed schema cache load --- CHANGELOG.md | 2 ++ src/PostgREST/App.hs | 2 +- src/PostgREST/Error.hs | 8 ++++---- test/io-tests/test_io.py | 5 +++++ 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60d8b03103..14a2e1de0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2077, Fix `is` not working with upper or mixed case values like `NULL, TrUe, FaLsE` - @steve-chavez - #2024, Fix schema cache loading when views with XMLTABLE and DEFAULT are present - @wolfgangwalther - #1724, Fix wrong CORS header Authentication -> Authorization - @wolfgangwalther + - #2107, Clarify error for failed schema cache load. - @steve-chavez + + From `Database connection lost. Retrying the connection` to `Could not query the database for the schema cache. Retrying.` ## [9.0.0] - 2021-11-25 diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 62dfd7ac31..f7c13d3e85 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -191,7 +191,7 @@ postgrestResponse conf maybeDbStructure jsonDbS pgVer pool time req = do Just dbStructure -> return dbStructure Nothing -> - throwError Error.ConnectionLostError + throwError Error.NoSchemaCacheError apiRequest@ApiRequest{..} <- liftEither . mapLeft Error.ApiRequestError $ diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index c3fc8c3a81..9dd737d4ba 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -280,7 +280,7 @@ data Error = GucHeadersError | GucStatusError | BinaryFieldError ContentType - | ConnectionLostError + | NoSchemaCacheError | PutMatchingPkError | PutRangeNotAllowedError | JwtTokenMissing @@ -294,7 +294,7 @@ instance PgrstError Error where status GucHeadersError = HTTP.status500 status GucStatusError = HTTP.status500 status (BinaryFieldError _) = HTTP.status406 - status ConnectionLostError = HTTP.status503 + status NoSchemaCacheError = HTTP.status503 status PutMatchingPkError = HTTP.status400 status PutRangeNotAllowedError = HTTP.status400 status JwtTokenMissing = HTTP.status500 @@ -317,8 +317,8 @@ instance JSON.ToJSON Error where "message" .= ("response.status guc must be a valid status code" :: Text)] toJSON (BinaryFieldError ct) = JSON.object [ "message" .= ((T.decodeUtf8 (ContentType.toMime ct) <> " requested but more than one column was selected") :: Text)] - toJSON ConnectionLostError = JSON.object [ - "message" .= ("Database connection lost. Retrying the connection." :: Text)] + toJSON NoSchemaCacheError = JSON.object [ + "message" .= ("Could not query the database for the schema cache. Retrying." :: Text)] toJSON PutRangeNotAllowedError = JSON.object [ "message" .= ("Range header and limit/offset querystring parameters are not allowed for PUT" :: Text)] diff --git a/test/io-tests/test_io.py b/test/io-tests/test_io.py index 9a16380f97..ed6c0b55ac 100644 --- a/test/io-tests/test_io.py +++ b/test/io-tests/test_io.py @@ -787,12 +787,17 @@ def test_admin_ready_includes_schema_cache_state(defaultenv): "/rpc/no_schema_cache_for_limited_authenticator" ) assert response.status_code == 200 + # force a reconnection so the new role setting is picked up postgrest.process.send_signal(signal.SIGUSR1) time.sleep(0.1) + response = postgrest.admin.get("/ready") assert response.status_code == 503 + response = postgrest.session.get("/projects") + assert response.status_code == 503 + def test_admin_not_found(defaultenv): "Should get a not found from a undefined endpoint on the admin server"