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

@@ -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 `<host>:<admin_server_port>/live` endpoint. A 200 OK status will be returned if postgrest is alive, otherwise a 503 will be returned.
+   + A `<host>:<admin_server_port>/live` endpoint is available for checking if postgrest is running on its port/socket. 200 OK = alive, 503 = dead.
    + A `<host>:<admin_server_port>/ready` endpoint is available for checking a correct internal state(the database connection plus the schema cache). 200 OK = ready, 503 = not ready.
 
 ### Fixed

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
+      respond $ Wai.responseLBS (if isMainAppReachable && isConnectionUp && isSchemaCacheLoaded then HTTP.status200 else HTTP.status503) [] mempty
-        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
     ["live"] ->
       respond $ Wai.responseLBS (if isMainAppReachable then HTTP.status200 else HTTP.status503) [] mempty
     _ ->

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 -> Maybe DbStructure -> IO ()
-putDbStructure appState structure =
+putDbStructure appState = atomicWriteIORef (stateDbStructure appState)
-  atomicWriteIORef (stateDbStructure appState) $ Just structure
 
 getJsonDbS :: AppState -> IO ByteString
 getJsonDbS = readIORef . stateJsonDbS

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"

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 ())

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 ;

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"