This is an automated email from the ASF dual-hosted git repository. sbglasius pushed a commit to branch fix/embedded-mongo-replica-set-review-fixes in repository https://gitbox.apache.org/repos/asf/grails-core.git
commit bd099a892f7f1ab341b449016765d96055bfee2a Author: Søren Berg Glasius <[email protected]> AuthorDate: Sat Aug 22 11:50:30 2026 +0200 fix(mongodb): replace a running embedded server on restart, recover a lost port Two review findings on the embedded replica set change: - RunningMongod.restart() returned without doing anything when the server was still up, which is silent in the one case the method exists for: a restore whose checkpoint was taken without the lifecycle bean stopping the server first. The old process is now torn down and replaced, so the replacement is not fighting it for the port. - EmbeddedReplicaSetSpec set reuseAddress on a socket ServerSocket(0) had already bound, so it did nothing, and a port lost between the offer and mongod binding it failed the whole specification - only a server the asking JVM started is ever reused. The dead line is gone and the start is asked for again on a fresh port. EmbeddedMongoLifecycleSpec covers the restart; it fails against the guard it replaces. --- .../mongodb/embedded/FlapdoodleMongoBackend.java | 11 ++++-- .../embedded/EmbeddedMongoLifecycleSpec.groovy | 28 +++++++++++++++ .../testing/mongo/EmbeddedReplicaSetSpec.groovy | 40 ++++++++++++++++++---- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/grails-data-mongodb/embedded/src/main/java/org/grails/datastore/gorm/mongodb/embedded/FlapdoodleMongoBackend.java b/grails-data-mongodb/embedded/src/main/java/org/grails/datastore/gorm/mongodb/embedded/FlapdoodleMongoBackend.java index 7a9f7deaac..97ca6947aa 100644 --- a/grails-data-mongodb/embedded/src/main/java/org/grails/datastore/gorm/mongodb/embedded/FlapdoodleMongoBackend.java +++ b/grails-data-mongodb/embedded/src/main/java/org/grails/datastore/gorm/mongodb/embedded/FlapdoodleMongoBackend.java @@ -206,11 +206,18 @@ public class FlapdoodleMongoBackend implements EmbeddedMongoBackend { * The replacement binds the same port because {@code Net} was fixed when the server * was first configured. Only a persistent {@code database-dir} carries data across; * mongod is a separate process, so a checkpoint image does not contain it. + * + * <p>A server that is somehow still up is torn down first rather than left: the + * replacement binds the port the old process is holding, so starting one beside the + * other only fails on the port. Ordinarily there is nothing to tear down, because the + * lifecycle bean stops the server before the checkpoint that this restores from. */ @Override public synchronized void restart() { - if (this.running != null) { - return; + TransitionWalker.ReachedState<RunningMongodProcess> current = this.running; + if (current != null) { + this.running = null; + current.close(); } this.running = this.mongod.start(this.version); initiateReplicaSet(); diff --git a/grails-data-mongodb/embedded/src/test/groovy/org/grails/datastore/gorm/mongodb/embedded/EmbeddedMongoLifecycleSpec.groovy b/grails-data-mongodb/embedded/src/test/groovy/org/grails/datastore/gorm/mongodb/embedded/EmbeddedMongoLifecycleSpec.groovy index 6bc2d0439d..464bfbea46 100644 --- a/grails-data-mongodb/embedded/src/test/groovy/org/grails/datastore/gorm/mongodb/embedded/EmbeddedMongoLifecycleSpec.groovy +++ b/grails-data-mongodb/embedded/src/test/groovy/org/grails/datastore/gorm/mongodb/embedded/EmbeddedMongoLifecycleSpec.groovy @@ -153,6 +153,28 @@ class EmbeddedMongoLifecycleSpec extends Specification { conditions.eventually { assert !listening(27979) } } + void 'a running server is replaced by a restart rather than fought with for its port'() { + given: 'a real mongod told to keep its data, so a replacement can be seen holding it' + String databaseDir = temp.resolve('restartDb').toString() + RunningEmbeddedMongo running = new FlapdoodleMongoBackend() + .start(new EmbeddedMongoSettings(27977, null, databaseDir)) + String url = 'mongodb://localhost:27977/bookstore' + write(url, 'Grails in Action') + long before = processId(url) + + when: 'a restore starts a server that whatever took the checkpoint never stopped' + running.restart() + + then: 'the port was released and taken again, rather than bound twice or left alone' + conditions.eventually { assert processId(url) != before } + + and: + titles(url) == ['Grails in Action'] + + cleanup: + running?.stop() + } + void 'an in-memory server is stopped twice by the same pair'() { given: RunningEmbeddedMongo running = new InMemoryMongoBackend() @@ -209,6 +231,12 @@ class EmbeddedMongoLifecycleSpec extends Specification { } } + private static long processId(String url) { + try (MongoClient client = MongoClients.create(url)) { + client.getDatabase('admin').runCommand(new Document('serverStatus', 1)).get('pid') as long + } + } + private static List<String> titles(String url) { try (MongoClient client = MongoClients.create(url)) { client.getDatabase('bookstore').getCollection('books').find()*.getString('title') diff --git a/grails-testing-support-mongodb/src/main/groovy/org/apache/grails/testing/mongo/EmbeddedReplicaSetSpec.groovy b/grails-testing-support-mongodb/src/main/groovy/org/apache/grails/testing/mongo/EmbeddedReplicaSetSpec.groovy index 195573725d..f438de6edb 100644 --- a/grails-testing-support-mongodb/src/main/groovy/org/apache/grails/testing/mongo/EmbeddedReplicaSetSpec.groovy +++ b/grails-testing-support-mongodb/src/main/groovy/org/apache/grails/testing/mongo/EmbeddedReplicaSetSpec.groovy @@ -46,17 +46,43 @@ abstract class EmbeddedReplicaSetSpec extends Specification { @Shared protected String mongoUrl + /** + * A port that is free when it is offered can be taken before mongod binds it, because the + * specifications using this run in forks beside each other and each fork asks the same + * operating system for the same range. The loser sees the start fail rather than the port + * quietly shared - only a server the asking JVM started is ever reused - so asking again is + * what recovers it. + */ + private static final int START_ATTEMPTS = 3 + void setupSpec() { - int port = freePort() - this.embedded = new GenericApplicationContext() - this.embedded.environment.propertySources.addFirst(new MapPropertySource('embeddedMongoTest', [ + IllegalStateException portLost = null + for (int attempt = 0; attempt < START_ATTEMPTS; attempt++) { + GenericApplicationContext context = contextOnPort(freePort()) + try { + new EmbeddedMongoInitializer().initialize(context) + } + catch (IllegalStateException failedToStart) { + portLost = failedToStart + context.close() + continue + } + this.embedded = context + this.mongoUrl = context.environment.getProperty('grails.mongodb.url') + return + } + throw portLost + } + + private static GenericApplicationContext contextOnPort(int port) { + GenericApplicationContext context = new GenericApplicationContext() + context.environment.propertySources.addFirst(new MapPropertySource('embeddedMongoTest', [ (EmbeddedMongoInitializer.BACKEND): FlapdoodleMongoBackend.NAME, (EmbeddedMongoInitializer.REPLICA_SET): 'rs0', (EmbeddedMongoInitializer.VERSION): serverVersion(), 'grails.mongodb.url': "mongodb://${EmbeddedMongoInitializer.EMBEDDED_HOST}:${port}/myDb".toString(), ])) - new EmbeddedMongoInitializer().initialize(this.embedded) - this.mongoUrl = this.embedded.environment.getProperty('grails.mongodb.url') + context } /** @@ -77,11 +103,11 @@ abstract class EmbeddedReplicaSetSpec extends Specification { /** * Asked for rather than fixed, because the specifications that use this run beside each other in - * a build: a port named in advance is a port another fork may hold. + * a build: a port named in advance is a port another fork may hold. The socket is closed before + * the port is handed on, which is a race {@link #START_ATTEMPTS} covers rather than avoids. */ private static int freePort() { new ServerSocket(0).withCloseable { ServerSocket socket -> - socket.reuseAddress = true socket.localPort } }
