Skip to content

Commit

Permalink
fix: slow responses on schema cache reload
Browse files Browse the repository at this point in the history
  • Loading branch information
steve-chavez committed Mar 15, 2024
1 parent 210cded commit 727ef46
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #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
- #3327, Fix slow responses on schema cache reloads - @steve-chavez

### Deprecated

Expand Down
4 changes: 3 additions & 1 deletion src/PostgREST/AppState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,12 @@ loadSchemaCache appState observer = do
return SCOnRetry

Right sCache -> do
putSchemaCache appState $ Just sCache
observer $ SchemaCacheQueriedObs resultTime
(t, _) <- timeItT $ observer $ SchemaCacheSummaryObs sCache
observer $ SchemaCacheLoadedObs t
-- it's important to update AppState schema cache only once it has been fully evaluated before (this will be done by the observer above)
-- otherwise requests will wait for the schema cache to be loaded
putSchemaCache appState $ Just sCache
putSchemaCacheLoaded appState True
return SCLoaded

Expand Down
4 changes: 4 additions & 0 deletions test/io/big_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -11384,3 +11384,7 @@ GRANT USAGE ON SCHEMA apflora TO postgrest_test_anonymous;

GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA apflora
TO postgrest_test_anonymous;

create or replace function apflora.notify_pgrst() returns void as $$
notify pgrst;
$$ language sql;
17 changes: 11 additions & 6 deletions test/io/test_big_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_first_request_succeeds(defaultenv):
"PGRST_SERVER_TIMING_ENABLED": "true",
}

with run(env=env, wait_max_seconds=20) as postgrest:
with run(env=env, wait_max_seconds=30) as postgrest:
response = postgrest.session.get("/tpopmassn?select=*,tpop(*)")
assert response.status_code == 200

Expand All @@ -27,8 +27,8 @@ def test_first_request_succeeds(defaultenv):
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"
def test_requests_do_not_wait_for_schema_cache_reload(defaultenv):
"requests that use the schema cache (e.g. resource embedding) do not wait for the schema cache to reload"

env = {
**defaultenv,
Expand All @@ -38,15 +38,20 @@ def test_requests_wait_for_schema_cache_to_be_loaded(defaultenv):
"PGRST_SERVER_TIMING_ENABLED": "true",
}

with run(env=env, wait_for_readiness=False) as postgrest:
time.sleep(1.5) # manually wait for schema cache to start loading
with run(env=env, wait_max_seconds=30) as postgrest:
# reload the schema cache
response = postgrest.session.get("/rpc/notify_pgrst")
assert response.status_code == 204

time.sleep(1.5) # wait for schema cache to start reloading

response = postgrest.session.get("/tpopmassn?select=*,tpop(*)")
assert response.status_code == 200

# response should be fast
server_timings = parse_server_timings_header(response.headers["Server-Timing"])
plan_dur = server_timings["plan"]
assert plan_dur > 10000.0
assert plan_dur < 2.0


# TODO: This test fails now because of https://github.com/PostgREST/postgrest/pull/2122
Expand Down

0 comments on commit 727ef46

Please sign in to comment.