fix: sqlx::macro db cleanup race condition by adding a margin to current timestamp

[fhsgoncalves]

Issue

As described in #2631, the #[sqlx::test] macro is occasionally failing to connect to the created database.

• The PR I submitted before, fix: race condition while cleaning up database on sqlx::test macro for postgres #2635, trying to fix the race condition to ensure the current timestamp was acquired in the proper order didn't cover all the cases
• So I realize that the issue may be that the SystemTime is not monotonic, as we can see from the documentation:

Distinct from the Instant type, this time measurement is not monotonic. This means that you can save a file to the file system, then save another file to the file system, and the second file has a SystemTime measurement earlier than the first. In other words, an operation that happens after another operation in real time may have an earlier SystemTime!

Solution

• The easiest way to fix this issue was to add a little margin (2 seconds) when comparing the timestamps for cleaning up the old databases.

WDYT of this approach? I'm testing here and it seems to have fixed the issue! The margin doesn't guarantee that the error won't occur again, but it decreases a lot the probability of it happening.

[abonander]

The real issue is with the multiprocess model of cargo-nextest, which this was not designed for, which has been a known issue: #2123

The existing code assumes all testing is happening in the same process, which is why it uses a static AtomicBool to decide whether to clean up or not.

You'd need some sort of multi-process lock to fix this properly. I had a brief look around for crates implementing something like that but didn't find anything I particularly liked:

• There's proc_lock which uses the fs2 crate, which hasn't been updated in 6 years.
• There's named_lock which uses flock() on UNIX, which has some pretty weird semantics: https://users.rust-lang.org/t/cross-platform-library-for-file-locking/68698/4

The POSIX specification has named semaphores but I can't find any crate wrapping it that's even worth considering. named_semaphore hasn't been updated in 2 years and its API is completely undocumented. sema also hasn't been updated in 6 years, and specifically doesn't do named semaphores.

[abonander]

Alternatively, we could cooperate with the nextest authors to add support for lifecycle hooks so we don't have to call the cleanup function within the tests themselves.

[fhsgoncalves]

That's interesting.

So using cargo test should always work? Because I'm experiencing the same issue when running with cargo test.
Even forcing just one job (with --jobs 1) I got the error frequently.

[gengjun]

That's interesting.

So using cargo test should always work? Because I'm experiencing the same issue when running with cargo test. Even forcing just one job (with --jobs 1) I got the error frequently.

yes, i can confirm even with just cargo test, two separate sqlx::test will fail or not fail randomly. My case is different, it's due to multiple tests sharing one global static PgPool.

[abonander]

I'll accept this as a marginal improvement but note that I would prefer some cooperation from nextest here.