fix: incorrect /ready response on slow schema load

CHANGELOG.md

@@ -23,6 +23,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #3160, Fix using select= query parameter for custom media type handlers - @wolfgangwalther
  - #3237, Dump media handlers and timezones with --dump-schema - @wolfgangwalther
  - #3323, #3324, Don't hide error on LISTEN channel failure - @steve-chavez
+ - #3330, Incorrect admin server `/ready` response on slow schema cache loads - @steve-chavez
 
 ### Deprecated
 

src/PostgREST/Admin.hs

@@ -38,7 +38,7 @@ runAdmin conf@AppConfig{configAdminServerPort} appState settings observer =
 admin :: AppState.AppState -> AppConfig -> (Observation -> IO ()) -> Wai.Application
 admin appState appConfig observer req respond  = do
   isMainAppReachable  <- isRight <$> reachMainApp (AppState.getSocketREST appState)
-  isSchemaCacheLoaded <- isJust <$> AppState.getSchemaCache appState
+  isSchemaCacheLoaded <- AppState.getSchemaCacheLoaded appState
   isConnectionUp      <-
     if configDbChannelEnabled appConfig
       then AppState.getIsListenerOn appState

src/PostgREST/AppState.hs

@@ -16,6 +16,7 @@ module PostgREST.AppState
   , getJwtCache
   , getSocketREST
   , getSocketAdmin
+  , getSchemaCacheLoaded
   , init
   , initSockets
   , initWithPool
@@ -85,6 +86,8 @@ data AppState = AppState
   , statePgVersion                :: IORef PgVersion
   -- | No schema cache at the start. Will be filled in by the connectionWorker
   , stateSchemaCache              :: IORef (Maybe SchemaCache)
+  -- | If schema cache is loaded
+  , stateSchemaCacheLoaded        :: IORef Bool
   -- | starts the connection worker with a debounce
   , debouncedConnectionWorker     :: IO ()
   -- | Binary semaphore used to sync the listener(NOTIFY reload) with the connectionWorker.
@@ -123,6 +126,7 @@ initWithPool (sock, adminSock) pool conf observer = do
   appState <- AppState pool
     <$> newIORef minimumPgVersion -- assume we're in a supported version when starting, this will be corrected on a later step
     <*> newIORef Nothing
+    <*> newIORef False
     <*> pure (pure ())
     <*> newEmptyMVar
     <*> newIORef False
@@ -283,6 +287,12 @@ getIsListenerOn = readIORef . stateIsListenerOn
 putIsListenerOn :: AppState -> Bool -> IO ()
 putIsListenerOn = atomicWriteIORef . stateIsListenerOn
 
+getSchemaCacheLoaded :: AppState -> IO Bool
+getSchemaCacheLoaded = readIORef . stateSchemaCacheLoaded
+
+putSchemaCacheLoaded :: AppState -> Bool -> IO ()
+putSchemaCacheLoaded = atomicWriteIORef . stateSchemaCacheLoaded
+
 -- | Schema cache status
 data SCacheStatus
   = SCLoaded
@@ -305,6 +315,7 @@ loadSchemaCache appState observer = do
         Nothing -> do
           putSchemaCache appState Nothing
           observer $ SchemaCacheNormalErrorObs e
+          putSchemaCacheLoaded appState False
           return SCOnRetry
 
     Right sCache -> do
@@ -312,6 +323,7 @@ loadSchemaCache appState observer = do
       observer $ SchemaCacheQueriedObs resultTime
       (t, _) <- timeItT $ observer $ SchemaCacheSummaryObs sCache
       observer $ SchemaCacheLoadedObs t
+      putSchemaCacheLoaded appState True
       return SCLoaded
 
 -- | Current database connection status data ConnectionStatus

test/io/postgrest.py

@@ -78,6 +78,7 @@ def run(
     port=None,
     host=None,
     wait_for_readiness=True,
+    wait_max_seconds=1,
     no_pool_connection_available=False,
     no_startup_stdout=True,
 ):
@@ -118,7 +119,7 @@ def run(
             process.stdin.close()
 
             if wait_for_readiness:
-                wait_until_ready(adminurl + "/ready")
+                wait_until_ready(adminurl + "/ready", wait_max_seconds)
 
             if no_startup_stdout:
                 process.stdout.read()
@@ -176,12 +177,11 @@ def wait_until_exit(postgrest):
         raise PostgrestTimedOut()
 
 
-def wait_until_ready(url):
+def wait_until_ready(url, max_seconds):
     "Wait for the given HTTP endpoint to return a status of 200."
     session = requests_unixsocket.Session()
 
-    response = None
-    for _ in range(10):
+    for _ in range(max_seconds * 10):
         try:
             response = session.get(url, timeout=1)
             if response.status_code == 200:

test/io/test_big_schema.py

@@ -7,6 +7,26 @@ from util import *
 from postgrest import *
 
 
+def test_first_request_succeeds(defaultenv):
+    "the first request suceeds fast since the admin server readiness was checked before"
+
+    env = {
+        **defaultenv,
+        "PGRST_DB_SCHEMAS": "apflora",
+        "PGRST_DB_POOL": "2",
+        "PGRST_DB_ANON_ROLE": "postgrest_test_anonymous",
+        "PGRST_SERVER_TIMING_ENABLED": "true",
+    }
+
+    with run(env=env, wait_max_seconds=20) as postgrest:
+        response = postgrest.session.get("/tpopmassn?select=*,tpop(*)")
+        assert response.status_code == 200
+
+        server_timings = parse_server_timings_header(response.headers["Server-Timing"])
+        plan_dur = server_timings["plan"]
+        assert plan_dur < 2.0
+
+
 def test_requests_wait_for_schema_cache_to_be_loaded(defaultenv):
     "requests that use the schema cache (e.g. resource embedding) wait for schema cache to be loaded"