fix: ensure embedded-postgres state is wiped between retries #20809
Conversation
|
Could this accidentally delete real postgres data? |
|
To Steven's question/your answer, I still think it would be a good idea to leave a Alternatively we could preemptively guard against that case by checking for I think the former (comment or add the additional check for |
Retries were previously added when starting embedded postgres to mitigate port allocation conflicts (we can't use an ephemeral port). Retries alone seemingly did not fix the test flakes. A new failure mode appeared on the retries: timing out connecting to the database. When a port discovery error occurrs, embedded-postgres does not create the database. If the data directory exists on the next attempt, embedded-postgres will assume the database has already been created. This seems to cause the timeout error. Wipe all state between retries to ensure attempts execute the same logic that creates the database.
9e8ff44 to
44948bc
Compare
|
Yeah, that's a really good point. I updated the comments to better document the intent and potential footgun to future folks that may change this code. |
Retries were previously added when starting embedded postgres to mitigate port allocation conflicts (we can't use an ephemeral port for tests). Retries alone seemingly did not fix the test flakes. A new failure mode appeared on the retries: timing out connecting to the database.
When a port discovery error occurrs, embedded-postgres does not create the database. If the data directory exists on the next attempt, embedded-postgres will assume the database has already been created. This seems to cause the timeout error. Wipe all state between retries to ensure attempts execute the same logic that creates the database.
#658
This isn't easily testable or reproducible. That said, I tested this on a Linux machine by removing the
testing.Testing()call that enables the retries, hard-coding the initial postgres port and usingncto force a port conflict.