Fix dropping schema cache reload notifications #2810
Merged
Conversation
This file contains 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
steve-chavez
force-pushed
the
2791
branch
4 times, most recently
from
00b4b7c to
c5d053b
Compare
steve-chavez
commented
Jun 2, 2023
test/io/test_io.py
Outdated
Comment on lines
951
to
984
|
|
||
| # If DB_POOL=1, then the second request(/rpc/migrate_function) will just wait(PGRST_DB_POOL_ACQUISITION_TIMEOUT=10) for the schema cache reload to finish. | ||
| # This is bc the only pool connection will be busy with the PGRST_INTERNAL_SCHEMA_CACHE_SLEEP(does a pg_sleep) | ||
| # So this must be tested with a DB_POOL size of at least 2. That way the second request will pick the other pool connection and proceed. | ||
|
|
||
| env = { | ||
| **defaultenv, | ||
| "PGRST_INTERNAL_SCHEMA_CACHE_SLEEP": "1", | ||
| "PGRST_DB_CHANNEL_ENABLED": "true", | ||
| "PGRST_DB_POOL": "2", | ||
| } | ||
|
|
||
| internal_sleep = int(env["PGRST_INTERNAL_SCHEMA_CACHE_SLEEP"]) | ||
|
|
||
| with run(env=env, wait_for_readiness=False) as postgrest: | ||
| time.sleep(internal_sleep + 0.1) # wait for readiness manually | ||
|
|
||
| response = postgrest.session.post("/rpc/create_function") | ||
| assert response.status_code == 204 | ||
|
|
||
| time.sleep( | ||
| internal_sleep / 2 | ||
| ) # wait to be inside the schema cache reload process | ||
|
|
||
| response = postgrest.session.post("/rpc/migrate_function") | ||
| assert response.status_code == 204 | ||
|
|
||
| time.sleep( | ||
| 2 * internal_sleep | ||
| ) # wait enough time to ensure the broken state remains | ||
|
|
||
| response = postgrest.session.get("/rpc/mult_them?c=3&d=4") | ||
| assert response.text == "12" | ||
| assert response.status_code == 200 |
There was a problem hiding this comment.
This was easy to test manually but really tricky in the io tests 😓
|
Tests that prove the failure: |
* Also rename Workers.hs to Admin.hs
steve-chavez
changed the title
|
Codecov failing bc of all the missing tests mentioned on #1766 |
steve-chavez
commented
Jun 2, 2023
| let oneSecond = 1000000 in | ||
| mkDebounce defaultDebounceSettings | ||
| { debounceAction = logPgrstError appState SQL.AcquisitionTimeoutUsageError | ||
| , debounceFreq = 5*oneSecond | ||
| , debounceEdge = leadingEdge -- logs at the start and the end | ||
| } | ||
|
|
||
| return appState { debounceLogAcquisitionTimeout = deb } | ||
| debWorker <- | ||
| let decisecond = 100000 in |
There was a problem hiding this comment.
This is enough to ensure schema cache reloading is still fast(io tests pass) but we still debounce.
Also tried manually with a bash loop for ((;;)) { kill -s SIGUSR1 $(pgrep -n postgrest); } and it's working fine.
|
Tests in main are flaky now. Will fix on a new PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #2791
Tasks
internal-schema-cache-sleepconfig that callspg_sleepat the end of the schema cache load.Notes
connectionWorker(and others) to the AppState module. It had to be done this way otherwise it caused a dependency cycle and compiling failed.