fix: Do not clear the schema cache during retries#4869
Open
mkleczek wants to merge 1 commit intoPostgREST:mainfrom
Open
fix: Do not clear the schema cache during retries#4869mkleczek wants to merge 1 commit intoPostgREST:mainfrom
mkleczek wants to merge 1 commit intoPostgREST:mainfrom
Conversation
mkleczek
force-pushed
the
push-olktzqvzkonu
branch
from
c468e80 to
36be579
Compare
Member
|
Clarifying the motivation on #4873 |
steve-chavez
reviewed
May 4, 2026
| - Restore Listener query shape so it can be found in pg_stat_activity by @mkleczek in #4857 #4859 | ||
| - The LISTEN channel now automatically recovers when it stops working due to a PostgreSQL bug @laurenceisla in #3147 | ||
| - Fix misleading "Functions" name on schema cache summary in startup logs by @taimoorzaeem in #4821 | ||
| - PostgREST no longer returns 503 error during schema cache loading retries by @mkleczek in #4869 |
Member
There was a problem hiding this comment.
Is this true? IIRC if the DB is down we will return 503 while schema cache retrying.
Collaborator
Author
There was a problem hiding this comment.
Yeah, that's too strong. Fixed.
mkleczek
force-pushed
the
push-olktzqvzkonu
branch
from
36be579 to
5201ea9
Compare
retryingSchemaCacheLoad should not clear existing schema cache upon failure - there is no reason to do that. If there is a communication issue with the database server or db is down, clients are going to get 502 anyway. If it was a glitch when loading the schema cache - the clients are going to use old (stale) schema cache for some time until next retry re-loads it successfully.
mkleczek
force-pushed
the
push-olktzqvzkonu
branch
from
5201ea9 to
82cf2e7
Compare
Member
|
Follow up question on #4873 (comment), not sure if it's correct to do the fix like this. |
| **defaultenv, | ||
| "PGUSER": role, | ||
| "PGRST_DB_ANON_ROLE": role, | ||
| "PGRST_DB_ANON_ROLE": "postgrest_test_anonymous", |
Member
There was a problem hiding this comment.
This seems like an unrelated fix for that test, which we should merge independently, so that we don't forget to do that even if we end up not doing the PR (as-is).
Is my interpretation correct?
| postgrest_test_serializable, postgrest_test_repeatable_read, | ||
| postgrest_test_w_superuser_settings TO :"PGUSER"; | ||
|
|
||
| GRANT postgrest_test_anonymous TO timeout_authenticator; |
Member
There was a problem hiding this comment.
This is not needed with the change to test_io.py, right?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DISCLAIMER:
This commit was authored entirely by a human without the assistance of LLMs.
retryingSchemaCacheLoadshould not clear existing schema cache upon failure - there is no reason to do that.If there is a communication issue with the database server or db is down, clients are going to get 502 anyway. If it was a glitch when loading the schema cache - the clients are going to use old (stale) schema cache for some time until next retry re-loads it successfully.
Fixes #4873