Skip to content

Commit

Permalink
Revert "fix: slow responses on schema cache reload"
Browse files Browse the repository at this point in the history
This reverts commit 727ef46.

Also documents requests waiting for the schema cache.
  • Loading branch information
steve-chavez committed Apr 3, 2024
1 parent d7c64a9 commit 3d55f77
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 29 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ 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
- #3340, Log when the LISTEN channel gets a notification - @steve-chavez
- #3345, Fix in-database configuration values not loading for `pgrst.server_trace_header` and `pgrst.server_cors_allowed_origins` - @laurenceisla
- #3361, Clarify PGRST204(column not found) error message - @steve-chavez
Expand Down
2 changes: 1 addition & 1 deletion docs/references/schema_cache.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ You can do this with UNIX signals or with PostgreSQL notifications. It's also po

.. note::

- Requests will wait until the schema cache reload is done. This to prevent client errors due to an stale schema cache.
- If you are using the :ref:`in_db_config`, a schema cache reload will :ref:`reload the configuration<config_reloading>` as well.
- There’s no downtime when reloading the schema cache. The reloading will happen on a background thread while serving requests.

.. _schema_reloading_signals:

Expand Down
4 changes: 1 addition & 3 deletions src/PostgREST/AppState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -320,12 +320,10 @@ 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
27 changes: 3 additions & 24 deletions test/io/test_big_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,8 @@
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=30) 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_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"
def test_requests__wait_for_schema_cache_reload(defaultenv):
"requests that use the schema cache (e.g. resource embedding) wait for the schema cache to reload"

env = {
**defaultenv,
Expand All @@ -48,10 +28,9 @@ def test_requests_do_not_wait_for_schema_cache_reload(defaultenv):
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 < 2.0
assert plan_dur > 10000.0


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

0 comments on commit 3d55f77

Please sign in to comment.