Skip to content

Commit

Permalink
fix: incorrect /ready response on slow schema load
Browse files Browse the repository at this point in the history
  • Loading branch information
steve-chavez committed Mar 15, 2024
1 parent 4b289b1 commit 3adf5bf
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/PostgREST/Admin.hs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions src/PostgREST/AppState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ module PostgREST.AppState
, getJwtCache
, getSocketREST
, getSocketAdmin
, getSchemaCacheLoaded
, init
, initSockets
, initWithPool
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -305,13 +315,15 @@ loadSchemaCache appState observer = do
Nothing -> do
putSchemaCache appState Nothing
observer $ SchemaCacheNormalErrorObs e
putSchemaCacheLoaded appState False
return SCOnRetry

Right sCache -> do
putSchemaCache appState $ Just sCache
observer $ SchemaCacheQueriedObs resultTime
(t, _) <- timeItT $ observer $ SchemaCacheSummaryObs sCache
observer $ SchemaCacheLoadedObs t
putSchemaCacheLoaded appState True
return SCLoaded

-- | Current database connection status data ConnectionStatus
Expand Down
8 changes: 4 additions & 4 deletions test/io/postgrest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
):
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 20 additions & 0 deletions test/io/test_big_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,26 @@
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"

Expand Down

0 comments on commit 3adf5bf

Please sign in to comment.