test: fix Spanner R2DBC flaky integration tests #2874
Conversation
| @AfterEach | ||
| void cleanupTestInstanceAfterTest() { | ||
| log.info("Deleting database {}", testDatabase); | ||
| this.spanner.getDatabaseAdminClient().dropDatabase(TEST_INSTANCE, testDatabase); |
There was a problem hiding this comment.
If something goes wrong and this cleanup isn't able to run, we'll have an orphaned database that never gets removed. Example of resolving this situation in google-cloud-java, by checking at the beginning of a test whether any stale resources exist from prior runs: https://github.com/googleapis/google-cloud-java/blob/main/java-compute/google-cloud-compute/src/test/java/com/google/cloud/compute/v1/integration/Util.java#L41-L57
There was a problem hiding this comment.
@AfterEach is guaranteed to run even if there is a test failure. Unlike Compute instances, Spanner databases don't incur additional costs, but there is a limit of 100 per instance. I would rather hold off the proactive additional cleanup that will slow down the tests, until it's clear that we need it.
There was a problem hiding this comment.
@AfterEach doesn't run if the machine fails - the exact problem we had with google-cloud-java. I leave it up to you.
| /** | ||
| * Abstract base for Spanner R2DBC integration tests of {@link SpannerR2dbcDialect}. | ||
| */ | ||
| abstract class AbstractBaseSpannerR2dbcIntegrationTest { |
There was a problem hiding this comment.
Test logs show a very large number of exceptions due to:
SEVERE: *~*~*~ Previous channel ManagedChannelImpl***logId=467, target=spanner.googleapis.com:443*** was garbage collected without being shut down! ~*~*~*
Make sure to call shutdown()/shutdownNow()
With long stack traces that pollute the results. Can we shut down the clients to avoid these errors?
|
Create a new database for each test for isolation.
Fixes: #2832.
Fixes: #2314.