graphql, indexer: introduce ephemeral postgres db for testing #19188
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
bmwill
force-pushed
the
indexer-async
branch
from
fb48c28 to
67c229a
Compare
Introduce LocalDatabase, a local instance of a postgres server, as well as TempDb, a way to create ephemeral postgres databases for use in testing enviornments.
Convert remaining graphql tests to use an ephemeral pstrgres db enabling them all to be run in parallel. After this patch the graphql-test workflow takes ~7 minutes compared to ~19 minutes it used to take.
bmwill
force-pushed
the
indexer-async
branch
from
4ee983a to
c7bf91e
Compare
bmwill
changed the title
bmwill
requested review from
amnn,
emmazzz,
stefan-mysten,
suiwombat and
wlmyng
as code owners
| cancellation_tokens.push(writer_token.drop_guard()); | ||
|
|
||
| // Start in reader mode | ||
| // Start indexer jsonrpc service |
There was a problem hiding this comment.
Been noticing this shift in terminology and big fan of it
| ReaderWriterConfig::writer_mode(None, None), | ||
| /* reset_database */ true, | ||
| Some(data_ingestion_path), | ||
| /* reset_database */ false, |
There was a problem hiding this comment.
Why the change to not reset db?
There was a problem hiding this comment.
so I think after this PR that flag is obsolete, as in we unconditionally reset the db when starting a writer in tests. This is contrary to this flags existence of course....which i think we specifically about deleting the DB in a shared postgres service.
| sim.advance_clock( | ||
| std::time::SystemTime::now() | ||
| .duration_since(std::time::SystemTime::UNIX_EPOCH) | ||
| .unwrap(), | ||
| ); |
There was a problem hiding this comment.
should we advance clock for each checkpoint to better emulate what happens in prod
There was a problem hiding this comment.
this is specifically to get the clock to be close to "current time" for the health check test. there's probably some changes we could make to Simulacrum to make this a bit easier of course
| /// Return an ephemeral, available port. On unix systems, the port returned will be in the | ||
| /// TIME_WAIT state ensuring that the OS won't hand out this port for some grace period. | ||
| /// Callers should be able to bind to this port given they use SO_REUSEADDR. | ||
| pub fn get_available_port() -> u16 { |
There was a problem hiding this comment.
throughout our tests we have a few places that do ("127.0.0.1:{}", get_available_port), what are your thoughts on having a function here that's like, build_db_address(host: Option<u64>) so we can reduce the manual 127.0.0.1
2 participants
Description
Convert all tests that relied on an external postgres db to use an ephemeral pstrgres db enabling them all to be run in parallel.
After this patch the graphql-test workflow takes ~7 minutes compared to ~19 minutes it used to take.
Test plan
How did you test the new or updated feature?
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.