Reuse an embedded MongoDB only when it is running the backend being asked for - #16216
Open
codeconsole wants to merge 1 commit into
Open
Reuse an embedded MongoDB only when it is running the backend being asked for#16216codeconsole wants to merge 1 commit into
codeconsole wants to merge 1 commit into
Conversation
A restart is handed the server this JVM already started when the settings it asks for match the ones it was started with. The backend was not among them, so switching from the in-memory reimplementation to flapdoodle -- or back -- while changing nothing else quietly kept the old server. The application asked for a real mongod and carried on against the in-memory one, which does not implement transactions, change streams or $text, so the failure arrives later and somewhere else. The check already compares the version, which the in-memory backend ignores entirely, so the omission was of the one setting that decides what the server actually is. The backend is resolved before the reuse question rather than inside start(), since it is part of that question, and the name it was started with is kept alongside the settings. A restart that asks for a backend whose library is absent now says so even when a server is already running, instead of being handed that server. That is the same answer it would have given had nothing been running.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 8.0.x #16216 +/- ##
==================================================
- Coverage 54.1238% 54.1139% -0.0099%
+ Complexity 20307 20301 -6
==================================================
Files 2107 2107
Lines 101144 101144
Branches 17921 17921
==================================================
- Hits 54743 54733 -10
- Misses 38595 38603 +8
- Partials 7806 7808 +2
🚀 New features to boost your workflow:
|
✅ All tests passed ✅🏷️ Commit: 1e74b1a Learn more about TestLens at testlens.app/docs. |
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
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.
Follow-up to #16192.
Problem
EmbeddedMongoInitializerhands a restart the server this JVM already started when the settings match the ones it was started with:EmbeddedMongoSettings.equalscomparesport,version,databaseDirandreplicaSet. The backend is not among them, andStartedServerdoes not carry it either, so nothing in the reuse path knows which backend is running.Switching
embedded.mongodb.backendbetweenin-memoryandflapdoodleacross a devtools restart, changing nothing else, therefore keeps the old server. An application that asked for a realmongodcarries on against the in-memory reimplementation, which implements neither transactions, change streams nor$text— so the failure surfaces later and somewhere else, as a missing feature rather than a backend that was never started.Worth noting the shape of the omission: the check does compare
version, whichInMemoryMongoBackendignores entirely. The one setting that decides what the server actually is was the one left out.Fix
Keep the backend the server was started with alongside its settings, and compare it:
The backend is resolved in
initializebefore the reuse question rather than insidestart, since it is part of that question, and passed tostartso it is selected once.EmbeddedMongoSettingsis public API and its constructor is unchanged; the backend lives on the privateStartedServerrecord instead.One behaviour change worth calling out: asking for a backend whose library is absent now fails even when a server is already running on that port, because
selectBackendruns before the reuse check. Previously such a restart was silently handed the running server. The new behaviour matches what the same configuration does when nothing is running.Tests
A restart that switches backend while holding version, database and replica set unchanged now gets a new server. The assertion is behavioural — a document written to the first server, then counted after the switch — because object identity does not distinguish the two cases here:
initializeconstructs a freshEmbeddedMongoLifecycleon every call, reused server or not.That last point applies to the existing test beside it.
a restart that asks for a different server is not handed the one already runningwas vacuous: I removedversionfromEmbeddedMongoSettings.equalsand it still passed, so it was not verifying replacement at all. It is rewritten here with the same behavioural assertion, and now fails whenversionis removed from the comparison.Both tests fail on unmodified
8.0.xand pass with the fix;:grails-data-mongodb-embedded:testis green.