Serve the transaction specifications from an embedded replica set - #16192
Merged
codeconsole merged 33 commits intoAug 24, 2026
Conversation
The lifecycle bean stops the server when the application context closes, and the JVM shutdown hook - which is there for a context that never closes - stops it again when the process exits. Flapdoodle removes the process directory as it tears the server down, so the second teardown failed on the files the first one had already deleted, and every run of an application or a test that uses a real mongod ended with a TearDownException on a shutdown thread. CI reports it as an error against a build whose tests all passed. Stopping the flapdoodle server is idempotent now, as stopping the in-memory one already was, and so is starting one that is already running. Both are covered.
MongoTransactionSpec fails about half the time in CI, and always the same way: the MongoDB container it talks to dies partway through the specification, and every test after the first reports a socket that ended prematurely. It is the only specification that flakes, and it runs beside three test JVMs, a MongoDB service container and whatever else the job started, each mongod sizing its cache from the memory of the whole machine. A transaction needs a replica set, which is the only reason these specifications reached for a container at all - so the embedded server learns to be one. Flapdoodle takes a replica set name on the command line; what was missing is the command that initiates the set and the wait for the node to elect itself, which is what makes a server that refuses every write into one that answers. An application asking GORM for transactions gets a replica set without naming one. The two transaction specifications now start that server instead of a container, on a port asked for rather than fixed. They need no Docker, so a developer without one runs them, and there is no container to lose halfway through.
codeconsole
force-pushed
the
fix/embedded-mongo-replica-set
branch
2 times, most recently
from
August 21, 2026 19:53
7a96972 to
3578a55
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16192 +/- ##
==================================================
+ Coverage 54.0934% 54.1149% +0.0215%
- Complexity 20258 20300 +42
==================================================
Files 2105 2107 +2
Lines 100992 101144 +152
Branches 17899 17921 +22
==================================================
+ Hits 54630 54734 +104
- Misses 38571 38602 +31
- Partials 7791 7808 +17
🚀 New features to boost your workflow:
|
The base specification that starts the server was in the core module's tests, where only the two specifications beside it could reach it. It moves to grails-testing-support-mongodb, which is the module both consumers already depend on and the one AutoStartedMongoSpec lives in, so one base class serves them rather than a copy in each. UnifiedMongoTransactionSpec and GormSpringDataSessionSupportSpec need a replica set for the same reason the core ones do, and now start the same embedded server rather than a container.
codeconsole
force-pushed
the
fix/embedded-mongo-replica-set
branch
from
August 21, 2026 21:03
3578a55 to
63982e5
Compare
codeconsole
requested review from
borinquenkid,
jamesfredley,
jdaugherty,
matrei and
sbglasius
August 21, 2026 22:17
Flapdoodle downloads a real MongoDB the first time a build asks for one and keeps it under ~/.embedmongo, which no job cached - so every one of them fetched it again from fastdl.mongodb.org. The specifications that moved onto an embedded replica set hand that download to more jobs than before, and a build should not depend on a third-party host being reachable to run its tests. The archive changes only when the version the backend asks for does, so it sits under the key the dependency jars already use.
CI runs the MongoDB functional tests twice, once for each supported server version, and passes the version as -PmongodbContainerVersion. A project property named on the command line reaches the Gradle JVM and stops there, while the tests read the version as a system property - so both halves of that matrix have been running the same server. The 7.0 job and the 8.0 job each pulled mongo:7.0.19 six times in the last green run on 8.0.x. The property is forwarded to every test task now, so a build that asks for a version gets it, and the matrix tests what it says it tests.
The specifications served from an embedded replica set took whatever version the backend defaults to, which is the newest supported line, while every specification served from a container takes the version the build asks for - so four of them were quietly exercising a different server from their neighbours. They ask for the same version now, which the embedded server understands: a build that asks for 8.0 starts a 8.0 mongod here as it starts a 8.0 container there, and a build that asks for nothing gets the same default as everything else.
SecuredControllerSpec fails now and then with the login page as the page it was asserting against, and no error on it - the browser was still showing the form while the specification read what it thought was the page behind it. login() returned as soon as it clicked submit, and pageSource is a plain read with none of the waiting Geb gives a content lookup. It waits for the browser to leave the login page now, which is what clicking submit started. Both applications that carry this page object have the same race; the second has not failed yet.
The acl application waits this way already, and it is the better test: the url says the browser has left the login page, where a title only says the page reads differently - and a page reads differently in another language.
clearGrailsWebRequest removes the web request from the request before unbinding it from the thread, and a container recycles a request as soon as it completes. An asynchronous task cleaning up after one that has completed threw on the first half and never reached the second, so the thread was left carrying a finished request - and, since the throw happened inside the task the container was running, it also took the response down. The removal is attempted, the unbinding always happens.
An application that renders from a promise fails a request now and then, and the functional tests that cover it flake with it: the response comes back 500 and the log carries "Calling [asyncStart()] is not valid for a request with Async state [DISPATCHING]". Attaching a callback to a promise looks up the promise decorators, and the one that binds a web request starts asynchronous processing as it is constructed. A promise that completes while its callback is being attached leaves the container delivering the result of the task that ran, and it refuses to start a second cycle on the same request - a refusal that used to escape and take the response down with it. A request past the point of taking another task is not decorated. A request that cannot process asynchronously at all still says so, which is a mistake worth reporting rather than a race: WebPromisesSpec covers that and keeps it. Reproduced with 2400 concurrent requests against the two asynchronous endpoints of the gsp-sitemesh3 application, which failed 3 to 5 times before and none after.
Catching an exception to steer control flow invites the obvious rewrite: ask the async manager whether it already holds a concurrent result, and skip the decorator when it does. That rewrite was measured and it is worse. The container can move from "no result" to "dispatching" between the answer and the call, so asking narrows the window instead of closing it: one request in 4800 still failed, where catching failed none in 9600. Comment only.
Contributor
|
My AI reviewed this and found two things worth a follow-up. Rather than leave them as prose I pushed a single commit on top of this branch — cherry-pick it if you agree with the reasoning: Branch: 1.
|
matrei
approved these changes
Aug 22, 2026
| * <p>Every reference to the MongoDB driver lives here, so a standalone embedded server needs | ||
| * nothing of it on the class path. | ||
| * | ||
| * @author Grails |
Contributor
Author
There was a problem hiding this comment.
Removed. Its seven neighbours in that module carried the same @author Grails placeholder from #16095, so those are gone too, as an isolated commit (e6fa4eb) - tag lines only, nothing else touched.
One is left in the repository, in grails-core/.../CompileStaticArtefactInjector.groovy, which is outside this PR.
Comment on lines
116
to
123
| // The server a test starts is chosen by the build, and the test reads it as a system | ||
| // property: a project property named on the command line reaches the Gradle JVM and stops | ||
| // there, so a matrix asking for one version and a test defaulting to another agree only by | ||
| // accident. CI passes -PmongodbContainerVersion for exactly this. | ||
| if (rootProject.hasProperty('mongodbContainerVersion')) { | ||
| testTask.systemProperty('mongodbContainerVersion', rootProject.property('mongodbContainerVersion')) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Should this be moved to the mongodb specific test-config files in the gradle directory?
Contributor
Author
There was a problem hiding this comment.
Moved, though to three files rather than two.
mongodb-test-config.gradle and mongodb-forked-test-config.gradle cover the grails-data-mongodb modules. They do not cover the example applications: grails-test-examples/mongodb/** apply functional-test-config.gradle, and they read mongodbContainerVersion as well - SpringBootStartMongoExtension and the test-data-service Application both do. Putting it only in the two mongodb configs would have sent those back to their hardcoded 7.0.19 while the matrix asked for 8.0, which is the thing that commit exists to fix. So it is in the functional config too, behind the isMongo flag that file already works out for the onlyMongodbTests gating.
Checked per task with -PmongodbContainerVersion=8.0: every grails-data-mongodb-* and grails-test-examples-mongodb-* test task now sees 8.0, while grails-core and the non-mongo example applications see nothing at all - which was your point.
A server started with a replica set name is initiated by a command sent to it, and the server is not handed over until it has elected itself primary. Neither step is certain: the command can be refused, the election can time out, the thread can be interrupted. Any of those left a mongod running that nothing could ever stop. The failure came out of a constructor, so the object never reached the registry that holds started servers, and with it went the shutdown hook and the lifecycle bean. A mongod is a child process that outlives the JVM which started it, so what was left behind held the port and the database directory against every later attempt, under a process id nobody had written down. The process is closed before the failure is passed on.
A devtools reload closes one application context and opens another in the same JVM, and the embedded server is meant to carry across. It did not, in two ways. The context that started the server stops it on the way out, so the context that follows finds it stopped. Its lifecycle bean assumed otherwise - the server was listening when the bean was made, and the bean remembered that rather than asking - so Spring, which skips whatever reports itself already running, never started it. The url published named a server that was not there. The bean now answers from the server. The server was also reused whatever it had been started with. A reload that switched transactions on, named another version, or moved the database directory was handed the server it already had, and found out later and further away: a transaction refused by a standalone server says nothing about the setting that asked for one. What was asked for is now kept beside the server, and a server that does not match is replaced. Stopping an in-memory server twice threw where the second stop is a shutdown hook, which has nowhere to report to and is documented as never throwing. It is now a no-op.
A transaction, a change stream and a causally consistent read are all refused by a standalone MongoDB, and asking GORM for transactions now gets a single-node replica set instead. That, the name it runs under, the driver it needs to be initiated, and what a reload carrying different settings does were all absent from the guide.
"Grails" is not an author, and a file added now mostly carries no such tag at all.
Handing the setting to every test task in the repository put a MongoDB concern in front of projects that have nothing to do with MongoDB. It belongs with the configuration those tests already share. The two mongodb test configs cover the modules. The example applications are not covered by them - they apply the functional test config, and read the same property to choose the server they start - so it goes there too, behind the flag that file already works out for deciding which projects a MongoDB run includes.
The same placeholder the replica set initiator carried, on the seven files beside it. Nothing else changes.
An application that asks GORM for transactions is given a replica set, and the backend that reimplements the wire protocol cannot be one. It started anyway: a standalone emulator, a url published for it, a context that refreshed, and a failure at the first transaction - the same shape of failure this module already refuses to produce for a database directory it cannot keep. The specification that covered replacing a server was written on top of that gap - it asked the in-memory backend for a replica set - so it now asks for a different version instead, which is a setting that backend does accept.
A reused server has usually been stopped by the context that started it, and starting it again was left to its lifecycle bean. Spring starts lifecycle beans once the context has refreshed, and a datastore builds its indexes while it is being constructed - before that, and over the network. The reload failed there, on a url that named a server nothing was listening on. Started where it is reused, so what is published is true when it is published.
stop() cleared the field before closing the process, so a close that failed left a mongod running that this object reported as stopped: nothing would stop it, and a restart would try to bind the port it still held. The field is cleared once the close has happened. A cleanup that fails while unwinding a replica set that never formed is also no longer reported in place of the failure that caused it.
The constructor closes the process it started when the replica set does not form. restart(), which starts the same server again after a checkpoint or a reload, did not - it left a mongod running with a set that never formed, holding the port against everything after it.
Whether a server is persistent, or a replica set, already treats an empty value as a value not given - "database-dir:" with nothing after it is how a property arrives as an empty string. The comparison that decides whether the running server is the one being asked for did not, so a key with nothing after it, added or removed between reloads, replaced a healthy server and dropped everything in it, while logging that the settings differed.
A hook was added for each server started, and never removed. A JVM that replaces a server - a reload with a setting changed - kept a hook for every server it had ever started, each holding the one it replaced. One hook now reads the registry as it runs, so it stops what is there and holds nothing that is not. A replacement also removes only the entry it replaced, rather than whatever now sits under that port.
cleanupSpec closed a context that had never been refreshed, and closing a context that was never active does nothing at all - no lifecycle stop, no server stopped. Every specification built on this left a real mongod running for the rest of the fork, several at a time across parallel forks. It is stopped through the bean the initializer registers instead. The port it asks for is also no longer described as reusable: setting that after the socket is bound does nothing, and the gap between reading a free port and binding it cannot be closed here.
A closure parameter tidied on the way past, in a change about MongoDB and flaky tests. It belongs to whoever is changing that block for its own sake.
A replacement keeps whatever is in a database-dir, except across a change of version: MongoDB refuses to start against a directory another release wrote. The guide promised the data survived.
Building the decorator stores a web request on the servlet request before asking the container to start an asynchronous cycle. Where the container refuses - the race this branch exists for - that half-built object stayed on the request with no asynchronous context ever assigned, and a later lookup would find it, decorate a promise with it, and dereference the context that was never there. What was on the request before is put back, and the refusal is logged rather than swallowed in silence. Covered by a specification that drives a request which supports asynchronous processing and refuses to start another cycle - the state a real container is in while it delivers a result.
codeconsole
force-pushed
the
fix/embedded-mongo-replica-set
branch
from
August 22, 2026 17:18
db58240 to
3532810
Compare
… a lost port restart() returned as soon as it found a server already running, so a caller asking a live server to restart got silence where the code before the guard would at least have failed on the port. The old process is torn down and a replacement started, shown by comparing the process id mongod reports across the call. The port these specifications run on is free when it is offered and bound seconds later, and the forks beside each other ask the same operating system for the same range. Three attempts, each on a fresh port. The line setting reuseAddress after the socket had already bound is gone; it did nothing. Cherry-picked from bd099a8 on fix/embedded-mongo-replica-set-review-fixes.
Two changes to the commit before this one, which was written against this branch as it stood earlier this morning. restart() tears the old server down through stop() rather than closing it inline. stop() clears its field only once the close has happened, so a close that throws no longer leaves a running mongod that this object reports as stopped. A start attempt that loses its port stops whatever it managed to start rather than closing its context. Closing a context that was never refreshed does nothing at all - the same no-op that was leaving a mongod running for every specification in the fork.
codeconsole
force-pushed
the
fix/embedded-mongo-replica-set
branch
from
August 22, 2026 17:20
3532810 to
d20c006
Compare
CodeNarc counts a catch block whose body is a comment as an empty one, and Code Style failed on both. The async one logs what it saw. The specification asks whether there is a server to stop rather than catching the answer that there is not, so there is nothing to catch at all.
Twenty-five servers were started and five stopped; the rest were left to the shutdown hook the initializer registers, as the class said. That only works if the JVM can exit, and a Gradle test worker holding a live server cannot exit while any of its threads is not a daemon - the hook that would stop the server runs on the way out. The worker waits for what only its own exit would release, Gradle waits for the worker, and a build whose tests have all passed never finishes. It killed a Neo4j job twice: every task complete, no BUILD SUCCESSFUL, six hours of silence to the minute, and at the kill twenty-two orphan processes with three mongods among them, started five and a half hours earlier. The run before it, on the commit before, ended with two orphans and no mongod - the difference is which side of the race the workers landed on. Every context the specification builds is now one it stops.
✅ All tests passed ✅🏷️ Commit: 2c00340 Learn more about TestLens at testlens.app/docs. |
This was referenced Aug 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the
MongoTransactionSpecfailures that take out "Build Grails with Groovy snapshot (shard 1)"about half the time. The MongoDB container those specifications talk to dies partway through, and a
transaction needs a replica set, which is the only reason they reached for a container.
An embedded replica set
embedded.mongodb.replica-set=rs0Flapdoodle takes the name on the command line; what was missing is
replSetInitiateand the wait forthe node to elect itself, which turns a server that refuses every write into one that answers. An
application that asks GORM for transactions gets a replica set without naming one:
grails.mongodb.transactional=trueMongoTransactionSpec,MongoTransactionDisabledSpec,UnifiedMongoTransactionSpecandGormSpringDataSessionSupportSpecstart that server throughEmbeddedReplicaSetSpecinstead of acontainer, on a port asked for rather than fixed. No Docker, and no container to lose halfway
through. The
in-memorybackend refuses a replica set rather than starting a standalone server thatfails at the first transaction. Both are documented in the embedded MongoDB guide.
The version the matrix asks for
-PmongodbContainerVersionreached the Gradle JVM and stopped there, somongodb-version: ['7.0','8.0']ran 7.0.19 twice. It is forwarded as a system property from the MongoDB andfunctional test configurations, and the embedded server takes the same version. CI also caches
~/.embedmongo, so mongod is downloaded once per branch rather than once per job.Two flaky tests
LoginPage.login()returned before the login had landed, so the page it read was sometimes still thelogin form. It waits for the url to leave
/login/.A callback attached to a promise that completed while it was being attached asked the container to
start a second asynchronous cycle on a request already delivering a result. The container refuses -
Calling [asyncStart()] is not valid for a request with Async state [DISPATCHING]- and the refusalfailed the response. Such a request is not decorated. Measured against the async endpoints of
gsp-sitemesh3: 3-5 failures per 2,400 requests before, none in 9,600 after.Server lifecycle
Stopping is idempotent, since the application context stops the server and the JVM shutdown hook
follows it. A server reused across a devtools reload is restarted where it is reused rather than left
to its lifecycle bean, which Spring starts only after the context has refreshed - a datastore builds
its indexes before that, over the network. A reload carrying different settings gets a server that
matches them. One shutdown hook per JVM rather than one per server.
What is not changed
The Mongo specifications that do not need a replica set stay on Testcontainers. This does not answer
why the container died; it removes the dependency on one.