pjfanning opened a new pull request, #610: URL: https://github.com/apache/pekko-persistence-jdbc/pull/610
Motivation: The SqlServer migrator integration tests failed on main in [run 34093026922](https://github.com/apache/pekko-persistence-jdbc/actions/runs/34093026922/job/101650378069) when the runner was slow: ``` - should migrate the event journal preserving the order of events *** FAILED *** (18s) The code passed to eventually never returned normally. Attempted 1 times over 10.007s. Last failure: A timeout occurred waiting for a future to complete. Waited 10000000000 ns. - should migrate the event journal preserving tags *** FAILED *** (8.9s) 262 was not equal to 0 ``` The commit under test (#607) does not touch journal migration and the same branch passed this job on its PR run, so this is a timing flake rather than a regression. There is no open issue tracking it. The second failure is fallout from the first. `withActorSystem` called `system.terminate()` after the test body rather than in a `finally`, so a failing body left the actor system running. Its in-flight migration stream kept writing while the next test recreated the tables in its `beforeEach`, and those 262 rows landed in the fresh table. One slow migration therefore produced two failures. The 10 second budget was also too tight. Comparing against the last green run of the same job (33907316903): | phase | green run | failing run | | --- | --- | --- | | legacy write phase (3000 events) | 6.3s | 8.4s | | migrate + verify phase | 7.3s | hit the 10.0s cap | | test total | 13.6s | 18.4s (failed) | Wrapping `migrate()` in `eventually` does not help either: the first `futureValue` consumes the whole patience window, so `eventually` never gets a second attempt, and a retry that did fire while the first migration was still running would insert the migrated rows twice. Modification: - Terminate the actor system in a `finally` block in both `withActorSystem` and `withLegacyActorSystem`. - Raise the migrator `PatienceConfig` timeout from 10 to 30 seconds. - Call `migrate()` once in `JournalMigratorTest` and keep `eventually` only around the row-count assertion that follows it. - Add `MigratorSpecFixtureTest`, which asserts that both fixtures terminate the actor system when the test body throws. It needs no database, so it runs anywhere. Result: A slow migration no longer corrupts the following test, and the migration is never run concurrently with itself. Tests: - `sbt "migratorIntegration/Test/testOnly org.apache.pekko.persistence.jdbc.migrator.H2JournalMigratorTest org.apache.pekko.persistence.jdbc.migrator.H2SnapshotMigratorTest org.apache.pekko.persistence.jdbc.migrator.MigratorSpecFixtureTest"` - 6 succeeded, 0 failed - `MigratorSpecFixtureTest` with the `finally` reverted - both tests fail (the actor system never terminates), confirming the fixture leak - `sbt "migratorIntegration/scalafmtAll"` and `sbt "migratorIntegration/headerCreateAll"` - run, no further changes - The SqlServer, Postgres, MySQL and Oracle migrator tests need running database servers and were left to CI References: Refs https://github.com/apache/pekko-persistence-jdbc/actions/runs/34093026922/job/101650378069, Refs #192 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
