You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "codeconsole (via GitHub)" <gi...@apache.org> on 2026/10/03 20:10:30 UTC
[PR] Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
codeconsole opened a new pull request, #16495:
URL: https://github.com/apache/grails-core/pull/16495
## Problem
With `-Dspring.context.checkpoint=onRefresh`, Spring takes the CRaC checkpoint as the context refreshes: after every bean has been created and before it starts any lifecycle bean. Before a checkpoint `DefaultLifecycleProcessor` stops only the beans that are running, and at that point none are, so nothing opened while beans were being created is closed. An application using GORM for MongoDB never got past it. The checkpoint failed, and the application did not start:
```
org.springframework.context.ApplicationContextException: Failed to take CRaC checkpoint on refresh
Caused by: org.crac.CheckpointException
Suppressed: jdk.internal.crac.mirror.impl.CheckpointOpenSocketException: Socket[addr=host.docker.internal/0.250.250.254,port=27017,localport=49302]
Suppressed: jdk.internal.crac.mirror.impl.CheckpointOpenSocketException: Socket[addr=host.docker.internal/0.250.250.254,port=27017,localport=49286]
```
That is an ordinary MongoDB server, not the embedded one. Two things connected while the beans were being created:
- `MongoConnectionSourceFactory` created the `MongoClient` as the datastore was built, and the driver starts the monitors that connect to the server as soon as a client exists.
- `MongoDatastore` built the indexes the domain classes declare in its constructor.
An embedded MongoDB added a third: `EmbeddedMongoInitializer` binds the server before the context refreshes, and `EmbeddedMongoLifecycle` is not stopped before this checkpoint either. With `-Dspring.context.exit=onRefresh`, Spring halts the JVM at the same point, which runs no shutdown hook, so a flapdoodle `mongod` was left running and holding its port.
A checkpoint of a running application already worked, but the client the application had been handed did not survive it. GORM closed every client it owned before the checkpoint and built replacements after the restore, and the `mongo` bean, which the guide shows injected into controllers and services, kept the closed one: every call on it after a restore failed with `IllegalStateException: state should be: open`.
## Change
- **`RestartableMongoClient`.** The `MongoClient` GORM creates for a connection is now a handle on a driver client that it builds when it is first used or started. `stop()` closes the driver client and refuses use until `start()` builds a new one, and `close()` is final. The handle stays the same object, so the `mongo` bean, a client obtained from `MongoDatastore.getMongoClient()`, multi-tenancy and the Spring Data integration go on working after a restore. `MongoConnectionSourceFactory`, the `Supplier<MongoClient>` constructor the Boot auto-configuration uses and the `MongoClientSettings.Builder` constructors all produce one. The connection settings are still built and checked as the datastore is created, so a URL that cannot be parsed is reported there. A client a custom connection source factory builds itself is still closed and replaced as before, and the factory's Javadoc says to wrap it instead.
- **`MongoDatastore` connects in `start()`.** The first start connects the client of every connection GORM owns and builds the declared indexes, in the datastore's lifecycle phase (`LIFECYCLE_PHASE`, -1000). Spring starts it before it publishes `ContextRefreshedEvent`, so the indexes are in place before `BootStrap` runs and before the web server accepts a request. A datastore that nothing starts, such as one created outside an application context, starts itself when the first session is opened on it, so code that creates one and queries it finds its indexes as before. A datastore that has been stopped is not started again by being used: its clients refuse until it is started, so a request arriving during a checkpoint cannot reopen a socket. `isRunning()` is `false` until the datastore has started.
- **`EmbeddedMongoInitializer`.** With `spring.context.checkpoint=onRefresh` or `spring.context.exit=onRefresh`, it publishes the URL and leaves starting the server to `EmbeddedMongoLifecycle`, which the context starts before the datastore (phase -2000). A URL asking for port `0` is given a free port when it is published, since the server binds later. Without either property nothing changes: the server is listening as soon as the initializer has run, so code that talks to MongoDB while beans are still being created finds it.
## Limits
A checkpoint taken as the context refreshes needs nothing else to have connected either. These still do, and the CRaC section of the guide names them:
- A `MongoClient` the application hands to GORM, including the `mongo` bean Spring Boot's own `MongoAutoConfiguration` defines, which the Boot auto-configuration passes to GORM rather than building a client itself. It is connected as it is created, and GORM neither starts nor stops a client it does not own.
- Application code that queries MongoDB while beans are being created, rather than in `BootStrap` or later.
- `MongoConnectionSources`, which reads the connections from MongoDB as the datastore is created.
A `MongoDatabase`, `MongoCollection` or `ClientSession` obtained from the client belongs to the driver client it came from, so one held across a checkpoint is closed with it. The client itself is what to keep.
## Tests
- `RestartableMongoClientSpec`: nothing is built until the client is used (naming, printing and comparing it are not uses); the first use builds one driver client and later uses share it; `start()` builds it at once; `stop()` closes it, use is refused with a message naming the connection, and `start()` serves the same handle from a new one; a client stopped before it was ever used builds nothing; `close()` is final; a driver client that fails to build leaves the handle untouched; sixteen threads using it for the first time at once share one driver client; and every method of `MongoClient` is passed through, checked by reflection against a recording driver client, so a method added to the driver's interface cannot be left out silently.
- `ConnectsWhenStartedSpec`: GORM registered through `MongoDbDataStoreSpringInitializer`, as a Grails application registers it, against a real server, with a driver listener on every client and a lifecycle bean at `Integer.MIN_VALUE`, which is where the checkpoint is taken. When that bean starts, no client has been created and no command sent; after the refresh the datastore is running, has issued `createIndexes`, and the domain class's index exists. It fails without the `MongoDatastore` change.
- `MongoDbDataStoreSpringInitializerSpec`: the `mongo` bean reaches MongoDB after the context is stopped and started again, as Spring does around a checkpoint, and is still the client GORM uses. It fails with `state should be: open` without the `RestartableMongoClient` change.
- `MongoDbGormAutoConfigurationCloseSpec`: the client the Boot auto-configuration builds from Spring Boot's settings does not exist when the first lifecycle bean starts, is created when the datastore starts, and stays the same client across a stop and start.
- `MongoDatastoreLifecycleSpec`: a datastore is not running and none of its clients, on any connection, is connected until it is started; the first session starts a datastore nothing has started; a stopped datastore is not started by a session opened on it; clients are restarted in place on every connection, including one added at runtime and a named connection beside an application-supplied default client; and a client a custom factory builds itself is still closed and replaced.
- `EmbeddedMongoStartedWithTheContextSpec`: with either property set, the URL is published and nothing is listening until the context starts; a lifecycle bean at `Integer.MIN_VALUE` finds nothing listening; port `0` is published as a real port; a port held by something else is reported when the server starts; and a server reused by a reloaded context is not started before that context starts it. Every feature fails without the initializer change. The properties are set as an application sets them, after `DefaultLifecycleProcessor` has been loaded, so the contexts the spec refreshes are not checkpointed or halted.
- The index-build specs that created a datastore and expected its indexes from the constructor now start it first, as the application context does. `BuildIndexesAsyncSpec` checks that the synchronous build runs on the thread that starts the datastore.
- All tests pass in `grails-data-mongodb-core`, `grails-data-mongodb-embedded`, `grails-data-mongodb-spring-boot`, `grails-data-mongodb-spring-data` and `grails-data-mongodb`, and in the `mongodb/base`, `mongodb/database-per-tenant` and `mongodb/springboot` test examples. Checkstyle and CodeNarc are clean for the changed modules. I have not run the whole `./gradlew build`.
Verified end to end with a Grails 8.0.0-RC2 application carrying these commits, on Azul Zulu CRaC 25, against MongoDB in Docker. Before, `-Dspring.context.checkpoint=onRefresh` failed with the exception above. With the change the checkpoint is taken before any client exists, the restore succeeds, the datastore connects as Spring restarts the lifecycle beans, `BootStrap` reads and updates a user, and logging in as that user works.
## Docs
- The guide's CRaC section covers a checkpoint taken as the context refreshes as well as one taken while the application runs, says that the client GORM hands out survives a restore (it used to say to obtain it again), and lists what still connects early.
- Index Creation on Startup and Building Indexes in the Background say when the datastore starts, and the Embedded MongoDB section says when its server is started under the two properties.
- What's New in the Grails guide has an entry for CRaC support in GORM for MongoDB.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4175735750
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1834,90 +1895,178 @@ public void persistentEntityAdded(PersistentEntity entity) {
*/
@Override
public void stop() {
- if (!this.running) {
- return;
- }
- List<MongoDatastore> datastores = datastoresAndChildren();
- List<MongoDatastore> owningTheirClient = new ArrayList<>();
- for (MongoDatastore datastore : datastores) {
- if (datastore.ownsClient()) {
- owningTheirClient.add(datastore);
+ synchronized (this.lifecycleMonitor) {
+ this.stopped = true;
+ List<MongoDatastore> datastores = datastoresAndChildren();
+ List<MongoDatastore> owningTheirClient = new ArrayList<>();
+ for (MongoDatastore datastore : datastores) {
+ if (datastore.ownsClient()) {
+ owningTheirClient.add(datastore);
+ }
}
+ if (owningTheirClient.isEmpty()) {
+ return;
+ }
+ for (MongoDatastore datastore : datastores) {
+ datastore.stopIndexBuild();
+ }
+ for (MongoDatastore datastore : owningTheirClient) {
+ datastore.stopClient();
+ }
+ this.running = false;
}
- if (owningTheirClient.isEmpty()) {
- return;
- }
- for (MongoDatastore datastore : datastores) {
- datastore.stopIndexBuild();
- }
- for (MongoDatastore datastore : owningTheirClient) {
- datastore.mongo.close();
- datastore.clientStopped = true;
- }
- this.running = false;
}
/**
- * Builds a replacement for each {@link MongoClient} that {@link #stop()} closed, using the
- * same factory the original was built with, so settings applied at startup still apply. The
- * replacement is handed out by the connection's {@link ConnectionSource} as well as by this
- * datastore, when that is the {@link MongoConnectionSource} the factory creates.
+ * Connects the datastore, which is the first point at which it opens a socket.
+ *
+ * <p>The first start connects the client of every connection GORM owns and builds the indexes the domain
+ * classes declare, unless {@code grails.mongodb.buildIndexes} is {@code false}. Nothing before it connects: the
+ * clients GORM creates are {@link RestartableMongoClient}s, which connect when first used, and building the
+ * datastore builds no index. So a datastore created while an application context refreshes holds no socket
+ * until Spring starts it in {@link #LIFECYCLE_PHASE}, which is what lets the process be checkpointed with CRaC as
+ * the context refreshes ({@code spring.context.checkpoint=onRefresh}). Spring starts it before it publishes
+ * {@code ContextRefreshedEvent}, so the indexes are in place before {@code BootStrap} runs and before the web
+ * server accepts a request. A datastore that nothing starts - one built outside an application context - starts
+ * itself the first time a session is opened on it.
*
- * <p>A background index build that {@link #stop()} cut short, or that was requested while stopped,
- * runs again on a fresh executor, on every connection.
+ * <p>Starting after {@link #stop()} brings back each {@link MongoClient} it stopped: a
+ * {@link RestartableMongoClient} builds a new driver client and stays the one handed out, and any other client is
+ * replaced by one built by the same factory the original was built with, so settings applied at startup still
+ * apply. A replacement is handed out by the connection's {@link ConnectionSource} as well as by this datastore,
+ * when that is a {@link MongoConnectionSource}. The indexes are not built again, since they outlive a checkpoint
+ * on the server, but a background index build that {@link #stop()} cut short, or that was requested while
+ * stopped, runs again on a fresh executor, on every connection. A connection added while the datastore was
+ * stopped is connected and has its indexes built here.
+ *
+ * <p>A connection registered while this runs is not left out. One registered by another thread waits for it to
+ * finish and is then connected and built as the running datastore's connections are; one registered by this
+ * thread, from an index build hook, is taken up by another pass before this returns.
*/
@Override
public void start() {
- if (this.running) {
+ synchronized (this.lifecycleMonitor) {
+ if (this.running) {
+ return;
+ }
+ this.stopped = false;
+ this.starting = true;
+ try {
+ startConnections();
+ // Only once every build has finished, or been handed to its thread: one that failed is tried again
+ // by the next start, or by the next use of a datastore that has never started.
+ this.started = true;
+ this.running = true;
+ }
+ finally {
+ this.starting = false;
+ }
+ }
+ }
+
+ /**
+ * Starts a datastore that nothing has started, the first time it is used. A datastore that has been stopped is
+ * not started here: whatever stopped it starts it again, and until then its clients refuse to be used.
+ */
+ void ensureStarted() {
+ if (this.started || this.closed || this.stopped) {
return;
}
+ synchronized (this.lifecycleMonitor) {
+ if (!this.started && !this.closed && !this.stopped && !this.starting) {
+ start();
+ }
+ }
+ }
+
+ /**
+ * Connects each client and runs each startup index build that is due, in passes, until a pass finds no
+ * datastore it has not already seen: an index build hook can register a connection while this runs.
+ */
+ private void startConnections() {
ConnectionSourceFactory<MongoClient, MongoConnectionSourceSettings> factory = connectionSources.getFactory();
- for (MongoDatastore datastore : datastoresAndChildren()) {
- if (!datastore.clientStopped) {
- continue;
+ Set<MongoDatastore> seen = Collections.newSetFromMap(new IdentityHashMap<>());
+ List<MongoDatastore> pass = datastoresAndChildren();
+ while (!pass.isEmpty()) {
+ seen.addAll(pass);
+ for (MongoDatastore datastore : pass) {
+ datastore.startClient(factory, datastore == this);
+ }
+ for (MongoDatastore datastore : pass) {
+ if (datastore.startupBuildDone) {
+ datastore.resumeIndexBuild();
+ }
+ else {
+ datastore.buildIndexAutomatically();
Review Comment:
You're right, thanks. Fixed in 89f1986198. `start()` now replaces a shut-down executor before either build, and the full build clears a pending build so it doesn't run twice. The exception is `grails.mongodb.buildIndexes = false`: there the full build builds nothing, so a `buildIndex()` requested while the datastore was stopped still has to run, and clearing it would have dropped that request.
`BuildIndexesLifecycleSpec` now runs the registered-while-stopped feature and a new stop-before-first-start feature both synchronously and with `buildIndexesAsync`, and checks that a later `buildIndex()` isn't put off either. Both async variants fail before the change, as you found. A third feature covers the `buildIndexes = false` case, and fails if the full build always clears the pending one.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#issuecomment-5975314995
## ✅ All tests passed ✅
🏷️ Commit: 9b6adc7f43d8cb7509f2ccdb3e912e50bdc01e05
▶️ Tests: 93984 executed
⚪️ Checks: 91/91 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4175845129
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -315,10 +318,29 @@ public void newConnectionSource(final ConnectionSource<MongoClient, MongoConnect
MongoDatastore childDatastore = createChildDatastore(mappingContext, eventPublisher, parent, singletonConnectionSources);
datastoresByConnectionSource.put(connectionSource.getName(), childDatastore);
registerAllEntitiesWithEnhancer();
- // Registered first and then checked: either close() has not started, and will find this
- // child when it walks the map, or it has, and the build is never started.
- if (!closed) {
- childDatastore.buildIndexAutomatically();
+ // Decided under the lifecycle monitor, so a connection registered while start() or stop() is
+ // under way waits for it and is then treated as the datastore now is.
+ synchronized (lifecycleMonitor) {
+ // Registered first and then checked: either close() has not started, and will find this
+ // child when it walks the map, or it has, and the build is never started.
+ if (closed) {
+ return;
+ }
+ if (running) {
+ // Unless the start() this waited for found it in the map, and has connected and
+ // built it already.
+ if (!childDatastore.startupBuildDone) {
+ childDatastore.startClient(connectionSources.getFactory(), false);
+ childDatastore.buildIndexAutomatically();
+ childDatastore.startupBuildDone = true;
+ }
+ }
+ else if (stopped && childDatastore.ownsClient()) {
+ // Refused, like the others, until start() connects it and builds its indexes.
+ childDatastore.stopClient();
Review Comment:
Non-blocking. A connection added while the datastore is stopped is the one case left where a `buildIndex()` requested while stopped is not deferred. `stop()` shut down every other connection's executor, but this child gets a fresh one. So with `grails.mongodb.buildIndexesAsync`, an explicit `buildIndex()` on it is submitted and runs at once against the stopped client. It logs `The background index build failed: The MongoClient for connection [added] is stopped ... The application is running without the indexes that were not created.` at ERROR, and records no pending build.
With `buildIndexes = true`, `start()` builds the connection anyway, so the only effect is the misleading error. With `buildIndexes = false`, the requested build is lost, while the same request on the default connection runs on `start()`. The `start()` javadoc says a build requested while stopped runs again on every connection.
I reproduced both at this head:
```groovy
def parent = new MongoDatastore(DatastoreUtils.createPropertyResolver([
'grails.mongodb.url' : url,
'grails.mongodb.buildIndexes' : false,
'grails.mongodb.buildIndexesAsync': true
]), Thing) // Thing is mapped with connection ConnectionSource.ALL
parent.start()
parent.stop()
parent.connectionSources.addConnectionSource('added', [url: addedUrl, buildIndexes: false])
parent.buildIndex() // deferred, runs on start()
(parent.getDatastoreForConnection('added') as MongoDatastore).buildIndex() // runs now and fails
parent.start() // 'added' never gets its index
```
Shutting the child's executor down along with its client fixes both. Now that `start()` replaces a shut-down executor whichever build it runs, that is one line here:
```suggestion
childDatastore.stopIndexBuild();
childDatastore.stopClient();
```
With that change both requests are deferred and run on `start()`, and `BuildIndexesLifecycleSpec`, `BuildIndexesPerConnectionSpec`, `BuildIndexesAsyncSpec` and `MongoDatastoreLifecycleSpec` still pass. The comment in `replaceStoppedIndexBuildExecutor()` that names this connection as the one whose executor was never shut down would need updating. A `buildIndex()` call in the added-while-stopped feature of `BuildIndexesPerConnectionSpec` would cover it.
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1834,90 +1895,178 @@ public void persistentEntityAdded(PersistentEntity entity) {
*/
@Override
public void stop() {
- if (!this.running) {
- return;
- }
- List<MongoDatastore> datastores = datastoresAndChildren();
- List<MongoDatastore> owningTheirClient = new ArrayList<>();
- for (MongoDatastore datastore : datastores) {
- if (datastore.ownsClient()) {
- owningTheirClient.add(datastore);
+ synchronized (this.lifecycleMonitor) {
+ this.stopped = true;
+ List<MongoDatastore> datastores = datastoresAndChildren();
+ List<MongoDatastore> owningTheirClient = new ArrayList<>();
+ for (MongoDatastore datastore : datastores) {
+ if (datastore.ownsClient()) {
+ owningTheirClient.add(datastore);
+ }
}
+ if (owningTheirClient.isEmpty()) {
+ return;
+ }
+ for (MongoDatastore datastore : datastores) {
+ datastore.stopIndexBuild();
+ }
+ for (MongoDatastore datastore : owningTheirClient) {
+ datastore.stopClient();
+ }
+ this.running = false;
}
- if (owningTheirClient.isEmpty()) {
- return;
- }
- for (MongoDatastore datastore : datastores) {
- datastore.stopIndexBuild();
- }
- for (MongoDatastore datastore : owningTheirClient) {
- datastore.mongo.close();
- datastore.clientStopped = true;
- }
- this.running = false;
}
/**
- * Builds a replacement for each {@link MongoClient} that {@link #stop()} closed, using the
- * same factory the original was built with, so settings applied at startup still apply. The
- * replacement is handed out by the connection's {@link ConnectionSource} as well as by this
- * datastore, when that is the {@link MongoConnectionSource} the factory creates.
+ * Connects the datastore, which is the first point at which it opens a socket.
+ *
+ * <p>The first start connects the client of every connection GORM owns and builds the indexes the domain
+ * classes declare, unless {@code grails.mongodb.buildIndexes} is {@code false}. Nothing before it connects: the
+ * clients GORM creates are {@link RestartableMongoClient}s, which connect when first used, and building the
+ * datastore builds no index. So a datastore created while an application context refreshes holds no socket
+ * until Spring starts it in {@link #LIFECYCLE_PHASE}, which is what lets the process be checkpointed with CRaC as
+ * the context refreshes ({@code spring.context.checkpoint=onRefresh}). Spring starts it before it publishes
+ * {@code ContextRefreshedEvent}, so the indexes are in place before {@code BootStrap} runs and before the web
+ * server accepts a request. A datastore that nothing starts - one built outside an application context - starts
+ * itself the first time a session is opened on it.
*
- * <p>A background index build that {@link #stop()} cut short, or that was requested while stopped,
- * runs again on a fresh executor, on every connection.
+ * <p>Starting after {@link #stop()} brings back each {@link MongoClient} it stopped: a
+ * {@link RestartableMongoClient} builds a new driver client and stays the one handed out, and any other client is
+ * replaced by one built by the same factory the original was built with, so settings applied at startup still
+ * apply. A replacement is handed out by the connection's {@link ConnectionSource} as well as by this datastore,
+ * when that is a {@link MongoConnectionSource}. The indexes are not built again, since they outlive a checkpoint
+ * on the server, but a background index build that {@link #stop()} cut short, or that was requested while
+ * stopped, runs again on a fresh executor, on every connection. A connection added while the datastore was
+ * stopped is connected and has its indexes built here.
+ *
+ * <p>A connection registered while this runs is not left out. One registered by another thread waits for it to
+ * finish and is then connected and built as the running datastore's connections are; one registered by this
+ * thread, from an index build hook, is taken up by another pass before this returns.
*/
@Override
public void start() {
- if (this.running) {
+ synchronized (this.lifecycleMonitor) {
+ if (this.running) {
+ return;
+ }
+ this.stopped = false;
+ this.starting = true;
+ try {
+ startConnections();
+ // Only once every build has finished, or been handed to its thread: one that failed is tried again
+ // by the next start, or by the next use of a datastore that has never started.
+ this.started = true;
+ this.running = true;
+ }
+ finally {
+ this.starting = false;
+ }
+ }
+ }
+
+ /**
+ * Starts a datastore that nothing has started, the first time it is used. A datastore that has been stopped is
+ * not started here: whatever stopped it starts it again, and until then its clients refuse to be used.
+ */
+ void ensureStarted() {
+ if (this.started || this.closed || this.stopped) {
return;
}
+ synchronized (this.lifecycleMonitor) {
+ if (!this.started && !this.closed && !this.stopped && !this.starting) {
+ start();
+ }
+ }
+ }
+
+ /**
+ * Connects each client and runs each startup index build that is due, in passes, until a pass finds no
+ * datastore it has not already seen: an index build hook can register a connection while this runs.
+ */
+ private void startConnections() {
ConnectionSourceFactory<MongoClient, MongoConnectionSourceSettings> factory = connectionSources.getFactory();
- for (MongoDatastore datastore : datastoresAndChildren()) {
- if (!datastore.clientStopped) {
- continue;
+ Set<MongoDatastore> seen = Collections.newSetFromMap(new IdentityHashMap<>());
+ List<MongoDatastore> pass = datastoresAndChildren();
+ while (!pass.isEmpty()) {
+ seen.addAll(pass);
+ for (MongoDatastore datastore : pass) {
+ datastore.startClient(factory, datastore == this);
+ }
+ for (MongoDatastore datastore : pass) {
+ if (datastore.startupBuildDone) {
+ datastore.resumeIndexBuild();
+ }
+ else {
+ datastore.buildIndexAutomatically();
Review Comment:
Thanks, verified at 89f1986198. Both cases now build their indexes with `buildIndexesAsync`, and a later `buildIndex()` is no longer deferred. Keeping the pending build when `buildIndexes = false` is right too.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#issuecomment-5973604400
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16495?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `92.23744%` with `17 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 59.4200%. Comparing base ([`657556a`](https://app.codecov.io/gh/apache/grails-core/commit/657556ac8de10d8de6eebb2134e0523d947c0be5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d1926a4`](https://app.codecov.io/gh/apache/grails-core/commit/d1926a446c79407ac23ea324c56653abd2d12861?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 12 commits behind head on 8.0.x.
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16495?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ping/mongo/connections/RestartableMongoClient.java](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fcore%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Fmongo%2Fconnections%2FRestartableMongoClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL21vbmdvL2Nvbm5lY3Rpb25zL1Jlc3RhcnRhYmxlTW9uZ29DbGllbnQuamF2YQ==) | 93.3333% | [3 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...grails/datastore/mapping/mongo/MongoDatastore.java](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fcore%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Fmongo%2FMongoDatastore.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL21vbmdvL01vbmdvRGF0YXN0b3JlLmphdmE=) | 91.2281% | [1 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../autoconfigure/MongoDbGormAutoConfiguration.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fboot-plugin%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Fmongodb%2Fboot%2Fautoconfigure%2FMongoDbGormAutoConfiguration.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9ib290LXBsdWdpbi9zcmMvbWFpbi9ncm9vdnkvb3JnL2dyYWlscy9kYXRhc3RvcmUvZ29ybS9tb25nb2RiL2Jvb3QvYXV0b2NvbmZpZ3VyZS9Nb25nb0RiR29ybUF1dG9Db25maWd1cmF0aW9uLmdyb292eQ==) | 88.0000% | [1 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...orm/mongodb/embedded/EmbeddedMongoInitializer.java](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fembedded%2Fsrc%2Fmain%2Fjava%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Fmongodb%2Fembedded%2FEmbeddedMongoInitializer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9lbWJlZGRlZC9zcmMvbWFpbi9qYXZhL29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vbW9uZ29kYi9lbWJlZGRlZC9FbWJlZGRlZE1vbmdvSW5pdGlhbGl6ZXIuamF2YQ==) | 93.3333% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16495 +/- ##
==================================================
+ Coverage 59.3728% 59.4200% +0.0472%
- Complexity 24833 24900 +67
==================================================
Files 2171 2172 +1
Lines 108186 108344 +158
Branches 19729 19755 +26
==================================================
+ Hits 64233 64378 +145
- Misses 35028 35033 +5
- Partials 8925 8933 +8
```
| [Files with missing lines](https://app.codecov.io/gh/apache/grails-core/pull/16495?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...go/connections/MongoConnectionSourceFactory.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fcore%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Fmongo%2Fconnections%2FMongoConnectionSourceFactory.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL21vbmdvL2Nvbm5lY3Rpb25zL01vbmdvQ29ubmVjdGlvblNvdXJjZUZhY3RvcnkuZ3Jvb3Z5) | `72.4138% <100.0000%> (+0.9852%)` | :arrow_up: |
| [.../autoconfigure/MongoDbGormAutoConfiguration.groovy](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fboot-plugin%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Fmongodb%2Fboot%2Fautoconfigure%2FMongoDbGormAutoConfiguration.groovy&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9ib290LXBsdWdpbi9zcmMvbWFpbi9ncm9vdnkvb3JnL2dyYWlscy9kYXRhc3RvcmUvZ29ybS9tb25nb2RiL2Jvb3QvYXV0b2NvbmZpZ3VyZS9Nb25nb0RiR29ybUF1dG9Db25maWd1cmF0aW9uLmdyb292eQ==) | `79.4118% <88.0000%> (+13.7868%)` | :arrow_up: |
| [...orm/mongodb/embedded/EmbeddedMongoInitializer.java](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fembedded%2Fsrc%2Fmain%2Fjava%2Forg%2Fgrails%2Fdatastore%2Fgorm%2Fmongodb%2Fembedded%2FEmbeddedMongoInitializer.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9lbWJlZGRlZC9zcmMvbWFpbi9qYXZhL29yZy9ncmFpbHMvZGF0YXN0b3JlL2dvcm0vbW9uZ29kYi9lbWJlZGRlZC9FbWJlZGRlZE1vbmdvSW5pdGlhbGl6ZXIuamF2YQ==) | `94.9367% <93.3333%> (-0.9649%)` | :arrow_down: |
| [...grails/datastore/mapping/mongo/MongoDatastore.java](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fcore%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Fmongo%2FMongoDatastore.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL21vbmdvL01vbmdvRGF0YXN0b3JlLmphdmE=) | `80.8861% <91.2281%> (+0.2028%)` | :arrow_up: |
| [...ping/mongo/connections/RestartableMongoClient.java](https://app.codecov.io/gh/apache/grails-core/pull/16495?src=pr&el=tree&filepath=grails-data-mongodb%2Fcore%2Fsrc%2Fmain%2Fgroovy%2Forg%2Fgrails%2Fdatastore%2Fmapping%2Fmongo%2Fconnections%2FRestartableMongoClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JhaWxzLWRhdGEtbW9uZ29kYi9jb3JlL3NyYy9tYWluL2dyb292eS9vcmcvZ3JhaWxzL2RhdGFzdG9yZS9tYXBwaW5nL21vbmdvL2Nvbm5lY3Rpb25zL1Jlc3RhcnRhYmxlTW9uZ29DbGllbnQuamF2YQ==) | `93.3333% <93.3333%> (ø)` | |
... and [5 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16495/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4175893539
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -315,10 +318,29 @@ public void newConnectionSource(final ConnectionSource<MongoClient, MongoConnect
MongoDatastore childDatastore = createChildDatastore(mappingContext, eventPublisher, parent, singletonConnectionSources);
datastoresByConnectionSource.put(connectionSource.getName(), childDatastore);
registerAllEntitiesWithEnhancer();
- // Registered first and then checked: either close() has not started, and will find this
- // child when it walks the map, or it has, and the build is never started.
- if (!closed) {
- childDatastore.buildIndexAutomatically();
+ // Decided under the lifecycle monitor, so a connection registered while start() or stop() is
+ // under way waits for it and is then treated as the datastore now is.
+ synchronized (lifecycleMonitor) {
+ // Registered first and then checked: either close() has not started, and will find this
+ // child when it walks the map, or it has, and the build is never started.
+ if (closed) {
+ return;
+ }
+ if (running) {
+ // Unless the start() this waited for found it in the map, and has connected and
+ // built it already.
+ if (!childDatastore.startupBuildDone) {
+ childDatastore.startClient(connectionSources.getFactory(), false);
+ childDatastore.buildIndexAutomatically();
+ childDatastore.startupBuildDone = true;
+ }
+ }
+ else if (stopped && childDatastore.ownsClient()) {
+ // Refused, like the others, until start() connects it and builds its indexes.
+ childDatastore.stopClient();
Review Comment:
Thanks, verified at e115da9b27. A `buildIndex()` on a connection added while stopped is now deferred and runs on `start()`, with `buildIndexes` on or off, and nothing logs an error. Both new features fail against the previous listener.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#issuecomment-5976600914
## ✅ All tests passed ✅
🏷️ Commit: e115da9b27e4cde0862222f1adb90d887828af35
▶️ Tests: 94522 executed
⚪️ Checks: 91/91 completed
---
_Learn more about TestLens at [testlens.app/docs](https://testlens.app/docs/features/pr-comment/)._
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4174981251
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1834,6 +1851,12 @@ public void persistentEntityAdded(PersistentEntity entity) {
*/
@Override
public void stop() {
+ synchronized (this.lifecycleMonitor) {
+ stopClients();
+ }
+ }
+
+ private void stopClients() {
if (!this.running) {
Review Comment:
`stop()` returns immediately when `running` is false, so a driver client opened before `start()` is not closed.
`getMongoClient()` or `buildIndex()` on a datastore that has not been started can already have created a driver client through `RestartableMongoClient`, while `running` is still false. A later `stop()`, including the one Spring takes before an on-refresh checkpoint, hits this return and leaves that socket open. The same hole exists if the first `start()` connects a client and then throws before it sets `running`.
Shutdown needs to close clients that have already been activated, even when startup has not completed, and refuse further use until an explicit `start()`. Please add a test that uses the client before `start()`, calls `stop()`, and asserts the driver is closed.
##########
grails-data-mongodb/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc:
##########
@@ -101,9 +101,15 @@ abstract class BookService {
Please note that with autowire by-type as the default, when multiple beans for same type are found the application with throw Exception. Use the Spring `@Qualifier annotation for https://docs.spring.io/spring-framework/docs/5.3.10/reference/html/core.html#beans-autowired-annotation-qualifiers[Fine-tuning Annotation Based Autowiring with Qualifiers].
+==== GORM Declares the MongoClient in a Spring Boot Application
Review Comment:
Non-blocking. This section covers Boot client ownership, but not the change that applies to every datastore: constructors no longer build indexes, including a constructor that is handed an application-owned client. Code that assumed a finished constructor meant the indexes existed needs an explicit note to call `start()`, or to do that work after lifecycle startup. The existing warning that querying during bean creation defeats an on-refresh checkpoint should stay.
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1851,51 +1874,102 @@ public void stop() {
datastore.stopIndexBuild();
}
for (MongoDatastore datastore : owningTheirClient) {
- datastore.mongo.close();
+ if (datastore.mongo instanceof RestartableMongoClient restartable) {
+ restartable.stop();
+ }
+ else {
+ datastore.mongo.close();
+ }
datastore.clientStopped = true;
}
this.running = false;
}
/**
- * Builds a replacement for each {@link MongoClient} that {@link #stop()} closed, using the
- * same factory the original was built with, so settings applied at startup still apply. The
- * replacement is handed out by the connection's {@link ConnectionSource} as well as by this
- * datastore, when that is the {@link MongoConnectionSource} the factory creates.
+ * Connects the datastore, which is the first point at which it opens a socket.
*
- * <p>A background index build that {@link #stop()} cut short, or that was requested while stopped,
- * runs again on a fresh executor, on every connection.
+ * <p>The first start connects the client of every connection GORM owns and builds the indexes the domain
+ * classes declare, unless {@code grails.mongodb.buildIndexes} is {@code false}. Nothing before it connects: the
+ * clients GORM creates are {@link RestartableMongoClient}s, which connect when first used, and building the
+ * datastore builds no index. So a datastore created while an application context refreshes holds no socket
+ * until Spring starts it in {@link #LIFECYCLE_PHASE}, which is what lets the process be checkpointed with CRaC as
+ * the context refreshes ({@code spring.context.checkpoint=onRefresh}). Spring starts it before it publishes
+ * {@code ContextRefreshedEvent}, so the indexes are in place before {@code BootStrap} runs and before the web
+ * server accepts a request. A datastore that nothing starts - one built outside an application context - starts
+ * itself the first time a session is opened on it.
+ *
+ * <p>Starting after {@link #stop()} brings back each {@link MongoClient} it stopped: a
+ * {@link RestartableMongoClient} builds a new driver client and stays the one handed out, and any other client is
+ * replaced by one built by the same factory the original was built with, so settings applied at startup still
+ * apply. A replacement is handed out by the connection's {@link ConnectionSource} as well as by this datastore,
+ * when that is a {@link MongoConnectionSource}. The indexes are not built again, since they outlive a checkpoint
+ * on the server, but a background index build that {@link #stop()} cut short, or that was requested while
+ * stopped, runs again on a fresh executor, on every connection.
*/
@Override
public void start() {
- if (this.running) {
+ synchronized (this.lifecycleMonitor) {
+ if (this.running) {
+ return;
+ }
+ if (this.started) {
+ restartClients();
+ }
+ else {
+ startForTheFirstTime();
+ }
+ }
+ }
+
+ /**
+ * Starts a datastore that nothing has started, the first time it is used. A datastore that has been stopped is
+ * not started here: whatever stopped it starts it again, and until then its clients refuse to be used.
+ */
+ void ensureStarted() {
+ if (this.started || this.closed) {
return;
}
+ synchronized (this.lifecycleMonitor) {
+ if (!this.started && !this.closed && !this.starting) {
+ start();
+ }
+ }
+ }
+
+ private void startForTheFirstTime() {
+ this.starting = true;
+ try {
+ List<MongoDatastore> datastores = datastoresAndChildren();
Review Comment:
A connection registered while this first startup is running can miss its indexes permanently. The children are snapshotted here, and `started` is set only after the build finishes. A connection that arrives in that window is not in the snapshot, and the listener at line 321 skips it because `started` is still false. Later sessions call `ensureStarted()` on the already-started parent, so the omitted build is never retried. Declared unique indexes can stay missing.
Coordinate registration with startup and drain connections that arrived during the first start before marking it complete. A regression test that registers a named connection after the snapshot, and asserts its indexes exist when startup returns, would lock this in.
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -316,8 +316,9 @@ public void newConnectionSource(final ConnectionSource<MongoClient, MongoConnect
datastoresByConnectionSource.put(connectionSource.getName(), childDatastore);
registerAllEntitiesWithEnhancer();
// Registered first and then checked: either close() has not started, and will find this
- // child when it walks the map, or it has, and the build is never started.
- if (!closed) {
+ // child when it walks the map, or it has, and the build is never started. A connection
+ // added before the datastore has started is built by start(), with the others.
+ if (!closed && started) {
Review Comment:
A connection added while the datastore is stopped still opens a socket. `started` stays true after `stop()`, so this calls `buildIndexAutomatically()` on a handle that is still `NEW`. That builds a driver client during the checkpoint window. A later parent `stop()` then returns immediately because `running` is already false, so the new socket is never closed.
Defer activation and index builds while the parent is stopped, and connect the new handle only on the next `start()`. `BuildIndexesPerConnectionSpec` currently expects the build to finish before restart. That expectation should be the opposite: no driver and no index commands while stopped, then indexes after restart.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4175882366
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -315,10 +318,29 @@ public void newConnectionSource(final ConnectionSource<MongoClient, MongoConnect
MongoDatastore childDatastore = createChildDatastore(mappingContext, eventPublisher, parent, singletonConnectionSources);
datastoresByConnectionSource.put(connectionSource.getName(), childDatastore);
registerAllEntitiesWithEnhancer();
- // Registered first and then checked: either close() has not started, and will find this
- // child when it walks the map, or it has, and the build is never started.
- if (!closed) {
- childDatastore.buildIndexAutomatically();
+ // Decided under the lifecycle monitor, so a connection registered while start() or stop() is
+ // under way waits for it and is then treated as the datastore now is.
+ synchronized (lifecycleMonitor) {
+ // Registered first and then checked: either close() has not started, and will find this
+ // child when it walks the map, or it has, and the build is never started.
+ if (closed) {
+ return;
+ }
+ if (running) {
+ // Unless the start() this waited for found it in the map, and has connected and
+ // built it already.
+ if (!childDatastore.startupBuildDone) {
+ childDatastore.startClient(connectionSources.getFactory(), false);
+ childDatastore.buildIndexAutomatically();
+ childDatastore.startupBuildDone = true;
+ }
+ }
+ else if (stopped && childDatastore.ownsClient()) {
+ // Refused, like the others, until start() connects it and builds its indexes.
+ childDatastore.stopClient();
Review Comment:
Thanks, fixed in e115da9b27. The listener now leaves a connection added while the datastore is stopped as `stop()` left the others: `stopIndexBuild()` always, and `stopClient()` only when GORM owns the client, the same check `stop()` uses. A `buildIndex()` on it is now put off and runs on `start()`. I updated the comment in `replaceStoppedIndexBuildExecutor()` that named this connection.
The added-while-stopped feature in `BuildIndexesPerConnectionSpec` now requests a build while stopped, checks it is put off, and checks nothing logs an error. A new feature covers `buildIndexes = false`: the requested build runs when the datastore starts. Both fail without the change.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "codeconsole (via GitHub)" <gi...@apache.org>.
codeconsole commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4175286833
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1834,6 +1851,12 @@ public void persistentEntityAdded(PersistentEntity entity) {
*/
@Override
public void stop() {
+ synchronized (this.lifecycleMonitor) {
+ stopClients();
+ }
+ }
+
+ private void stopClients() {
if (!this.running) {
Review Comment:
Fixed in b91ec8f76d. `stop()` no longer returns early when the datastore isn't running: it stops every client GORM owns whatever state the datastore is in. Until the next `start()`, those clients refuse to be used, and opening a session doesn't start the datastore. `MongoDatastoreLifecycleSpec` has the test you asked for: it uses the client before `start()`, calls `stop()`, and asserts the driver client is closed, use is refused, a session doesn't start the datastore, and `start()` reconnects it. A second test covers a first `start()` that connects and then fails on the index build. Both fail against the previous `stop()`.
One correction: Spring doesn't call `stop()` before an on-refresh checkpoint. It takes that checkpoint in `DefaultLifecycleProcessor.onRefresh()` before it starts any lifecycle bean, and its `beforeCheckpoint` stops beans only once the processor is running. So nothing closes a client something activates while beans are being created, and the CRaC section lists that among what defeats an on-refresh checkpoint. The hole is real for an explicit `stop()` and for a first start that fails, and both are covered now.
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -316,8 +316,9 @@ public void newConnectionSource(final ConnectionSource<MongoClient, MongoConnect
datastoresByConnectionSource.put(connectionSource.getName(), childDatastore);
registerAllEntitiesWithEnhancer();
// Registered first and then checked: either close() has not started, and will find this
- // child when it walks the map, or it has, and the build is never started.
- if (!closed) {
+ // child when it walks the map, or it has, and the build is never started. A connection
+ // added before the datastore has started is built by start(), with the others.
+ if (!closed && started) {
Review Comment:
Fixed in b91ec8f76d, as you suggested. The listener now decides under the lifecycle lock. A connection added while the datastore is stopped has its client stopped with the others, and nothing is built. The next `start()` connects it and builds its indexes. I reversed the `BuildIndexesPerConnectionSpec` expectation: while stopped there's no driver client, no index build logged for it, no index, and using it is refused; after `start()` it's connected and the index exists. A domain class registered while the datastore is stopped had the same problem and is deferred the same way, tested in `BuildIndexesLifecycleSpec`.
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1851,51 +1874,102 @@ public void stop() {
datastore.stopIndexBuild();
}
for (MongoDatastore datastore : owningTheirClient) {
- datastore.mongo.close();
+ if (datastore.mongo instanceof RestartableMongoClient restartable) {
+ restartable.stop();
+ }
+ else {
+ datastore.mongo.close();
+ }
datastore.clientStopped = true;
}
this.running = false;
}
/**
- * Builds a replacement for each {@link MongoClient} that {@link #stop()} closed, using the
- * same factory the original was built with, so settings applied at startup still apply. The
- * replacement is handed out by the connection's {@link ConnectionSource} as well as by this
- * datastore, when that is the {@link MongoConnectionSource} the factory creates.
+ * Connects the datastore, which is the first point at which it opens a socket.
*
- * <p>A background index build that {@link #stop()} cut short, or that was requested while stopped,
- * runs again on a fresh executor, on every connection.
+ * <p>The first start connects the client of every connection GORM owns and builds the indexes the domain
+ * classes declare, unless {@code grails.mongodb.buildIndexes} is {@code false}. Nothing before it connects: the
+ * clients GORM creates are {@link RestartableMongoClient}s, which connect when first used, and building the
+ * datastore builds no index. So a datastore created while an application context refreshes holds no socket
+ * until Spring starts it in {@link #LIFECYCLE_PHASE}, which is what lets the process be checkpointed with CRaC as
+ * the context refreshes ({@code spring.context.checkpoint=onRefresh}). Spring starts it before it publishes
+ * {@code ContextRefreshedEvent}, so the indexes are in place before {@code BootStrap} runs and before the web
+ * server accepts a request. A datastore that nothing starts - one built outside an application context - starts
+ * itself the first time a session is opened on it.
+ *
+ * <p>Starting after {@link #stop()} brings back each {@link MongoClient} it stopped: a
+ * {@link RestartableMongoClient} builds a new driver client and stays the one handed out, and any other client is
+ * replaced by one built by the same factory the original was built with, so settings applied at startup still
+ * apply. A replacement is handed out by the connection's {@link ConnectionSource} as well as by this datastore,
+ * when that is a {@link MongoConnectionSource}. The indexes are not built again, since they outlive a checkpoint
+ * on the server, but a background index build that {@link #stop()} cut short, or that was requested while
+ * stopped, runs again on a fresh executor, on every connection.
*/
@Override
public void start() {
- if (this.running) {
+ synchronized (this.lifecycleMonitor) {
+ if (this.running) {
+ return;
+ }
+ if (this.started) {
+ restartClients();
+ }
+ else {
+ startForTheFirstTime();
+ }
+ }
+ }
+
+ /**
+ * Starts a datastore that nothing has started, the first time it is used. A datastore that has been stopped is
+ * not started here: whatever stopped it starts it again, and until then its clients refuse to be used.
+ */
+ void ensureStarted() {
+ if (this.started || this.closed) {
return;
}
+ synchronized (this.lifecycleMonitor) {
+ if (!this.started && !this.closed && !this.starting) {
+ start();
+ }
+ }
+ }
+
+ private void startForTheFirstTime() {
+ this.starting = true;
+ try {
+ List<MongoDatastore> datastores = datastoresAndChildren();
Review Comment:
Fixed in b91ec8f76d, with a follow-up in 70517cf660. Registration now coordinates with startup through the lifecycle lock. `start()` works in passes: once it has connected and built every connection it knows of, it looks again, and it sets `started` only when a pass finds no connection it hasn't already handled. A connection another thread registers waits for `start()` and is connected and built when it gets the lock, unless `start()` already found it (70517cf660 stops it from being built a second time). One registered from an index build hook on the starting thread is picked up by the next pass. `BuildIndexesPerConnectionSpec` has a regression test for each, asserting the indexes exist when `start()` returns. Both fail against the previous `start()`.
##########
grails-data-mongodb/docs/src/docs/asciidoc/introduction/upgradeNotes.adoc:
##########
@@ -101,9 +101,15 @@ abstract class BookService {
Please note that with autowire by-type as the default, when multiple beans for same type are found the application with throw Exception. Use the Spring `@Qualifier annotation for https://docs.spring.io/spring-framework/docs/5.3.10/reference/html/core.html#beans-autowired-annotation-qualifiers[Fine-tuning Annotation Based Autowiring with Qualifiers].
+==== GORM Declares the MongoClient in a Spring Boot Application
Review Comment:
Added in 4987030acd. The GORM for MongoDB upgrade notes now have "Indexes Are Built When the Datastore Starts, Not When It Is Created". It covers every constructor, including one handed an application's own client, shows calling `start()`, and notes that the first GORM session starts a datastore nothing has started. The warning about querying during bean creation is still there. The Grails 8.0 upgrade notes have a matching bullet that links to it.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16495:
URL: https://github.com/apache/grails-core/pull/16495#discussion_r4175600869
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -1834,90 +1895,178 @@ public void persistentEntityAdded(PersistentEntity entity) {
*/
@Override
public void stop() {
- if (!this.running) {
- return;
- }
- List<MongoDatastore> datastores = datastoresAndChildren();
- List<MongoDatastore> owningTheirClient = new ArrayList<>();
- for (MongoDatastore datastore : datastores) {
- if (datastore.ownsClient()) {
- owningTheirClient.add(datastore);
+ synchronized (this.lifecycleMonitor) {
+ this.stopped = true;
+ List<MongoDatastore> datastores = datastoresAndChildren();
+ List<MongoDatastore> owningTheirClient = new ArrayList<>();
+ for (MongoDatastore datastore : datastores) {
+ if (datastore.ownsClient()) {
+ owningTheirClient.add(datastore);
+ }
}
+ if (owningTheirClient.isEmpty()) {
+ return;
+ }
+ for (MongoDatastore datastore : datastores) {
+ datastore.stopIndexBuild();
+ }
+ for (MongoDatastore datastore : owningTheirClient) {
+ datastore.stopClient();
+ }
+ this.running = false;
}
- if (owningTheirClient.isEmpty()) {
- return;
- }
- for (MongoDatastore datastore : datastores) {
- datastore.stopIndexBuild();
- }
- for (MongoDatastore datastore : owningTheirClient) {
- datastore.mongo.close();
- datastore.clientStopped = true;
- }
- this.running = false;
}
/**
- * Builds a replacement for each {@link MongoClient} that {@link #stop()} closed, using the
- * same factory the original was built with, so settings applied at startup still apply. The
- * replacement is handed out by the connection's {@link ConnectionSource} as well as by this
- * datastore, when that is the {@link MongoConnectionSource} the factory creates.
+ * Connects the datastore, which is the first point at which it opens a socket.
+ *
+ * <p>The first start connects the client of every connection GORM owns and builds the indexes the domain
+ * classes declare, unless {@code grails.mongodb.buildIndexes} is {@code false}. Nothing before it connects: the
+ * clients GORM creates are {@link RestartableMongoClient}s, which connect when first used, and building the
+ * datastore builds no index. So a datastore created while an application context refreshes holds no socket
+ * until Spring starts it in {@link #LIFECYCLE_PHASE}, which is what lets the process be checkpointed with CRaC as
+ * the context refreshes ({@code spring.context.checkpoint=onRefresh}). Spring starts it before it publishes
+ * {@code ContextRefreshedEvent}, so the indexes are in place before {@code BootStrap} runs and before the web
+ * server accepts a request. A datastore that nothing starts - one built outside an application context - starts
+ * itself the first time a session is opened on it.
*
- * <p>A background index build that {@link #stop()} cut short, or that was requested while stopped,
- * runs again on a fresh executor, on every connection.
+ * <p>Starting after {@link #stop()} brings back each {@link MongoClient} it stopped: a
+ * {@link RestartableMongoClient} builds a new driver client and stays the one handed out, and any other client is
+ * replaced by one built by the same factory the original was built with, so settings applied at startup still
+ * apply. A replacement is handed out by the connection's {@link ConnectionSource} as well as by this datastore,
+ * when that is a {@link MongoConnectionSource}. The indexes are not built again, since they outlive a checkpoint
+ * on the server, but a background index build that {@link #stop()} cut short, or that was requested while
+ * stopped, runs again on a fresh executor, on every connection. A connection added while the datastore was
+ * stopped is connected and has its indexes built here.
+ *
+ * <p>A connection registered while this runs is not left out. One registered by another thread waits for it to
+ * finish and is then connected and built as the running datastore's connections are; one registered by this
+ * thread, from an index build hook, is taken up by another pass before this returns.
*/
@Override
public void start() {
- if (this.running) {
+ synchronized (this.lifecycleMonitor) {
+ if (this.running) {
+ return;
+ }
+ this.stopped = false;
+ this.starting = true;
+ try {
+ startConnections();
+ // Only once every build has finished, or been handed to its thread: one that failed is tried again
+ // by the next start, or by the next use of a datastore that has never started.
+ this.started = true;
+ this.running = true;
+ }
+ finally {
+ this.starting = false;
+ }
+ }
+ }
+
+ /**
+ * Starts a datastore that nothing has started, the first time it is used. A datastore that has been stopped is
+ * not started here: whatever stopped it starts it again, and until then its clients refuse to be used.
+ */
+ void ensureStarted() {
+ if (this.started || this.closed || this.stopped) {
return;
}
+ synchronized (this.lifecycleMonitor) {
+ if (!this.started && !this.closed && !this.stopped && !this.starting) {
+ start();
+ }
+ }
+ }
+
+ /**
+ * Connects each client and runs each startup index build that is due, in passes, until a pass finds no
+ * datastore it has not already seen: an index build hook can register a connection while this runs.
+ */
+ private void startConnections() {
ConnectionSourceFactory<MongoClient, MongoConnectionSourceSettings> factory = connectionSources.getFactory();
- for (MongoDatastore datastore : datastoresAndChildren()) {
- if (!datastore.clientStopped) {
- continue;
+ Set<MongoDatastore> seen = Collections.newSetFromMap(new IdentityHashMap<>());
+ List<MongoDatastore> pass = datastoresAndChildren();
+ while (!pass.isEmpty()) {
+ seen.addAll(pass);
+ for (MongoDatastore datastore : pass) {
+ datastore.startClient(factory, datastore == this);
+ }
+ for (MongoDatastore datastore : pass) {
+ if (datastore.startupBuildDone) {
+ datastore.resumeIndexBuild();
+ }
+ else {
+ datastore.buildIndexAutomatically();
Review Comment:
With `grails.mongodb.buildIndexesAsync = true`, this branch builds on an executor that `stop()` shut down. Only `resumeIndexBuild()` replaces that executor, and it runs only on the `startupBuildDone` branch above. `buildIndex()` then finds the executor shut down, marks the build pending, and logs "An index build was requested for connection [default] while the datastore is stopped; it will run when the datastore is restarted", but the datastore is already running. The indexes are not built, and every later `buildIndex()` on that connection is deferred the same way until the next stop/start cycle.
This PR introduced two ways to reach this branch with a shut-down executor:
- A domain class registered while the datastore is stopped. `persistentEntityAdded` resets `startupBuildDone` on a datastore that has started.
- `stop()` before the first `start()`, which b91ec8f76d made an explicit path.
I reproduced both at this head, as the async counterpart of the new `BuildIndexesLifecycleSpec` feature:
```groovy
Map config = ['grails.mongodb.url': url, (MongoSettings.SETTING_BUILD_INDEXES_ASYNC): true]
// registered while stopped
def datastore = new MongoDatastore(config)
datastore.start()
datastore.stop()
datastore.mappingContext.addPersistentEntity(IndexedThing)
datastore.start() // IndexedThing's index is never built
// stopped before the first start
def other = new MongoDatastore(config, IndexedThing)
other.stop()
other.start() // never built either
```
Replacing a shut-down executor before the full build fixes both. For example, take the executor replacement out of `resumeIndexBuild()` and call it on both branches. Clearing `indexBuildPending` there as well stops the full build from running twice. With that change, both cases pass locally, as do `BuildIndexesLifecycleSpec`, `MongoDatastoreLifecycleSpec`, `BuildIndexesPerConnectionSpec` and `BuildIndexesAsyncSpec`. Please add async variants of the registered-while-stopped and stop-before-start tests.
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] 8.0 (fix): Let a GORM for MongoDB application be checkpointed as its context refreshes [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #16495:
URL: https://github.com/apache/grails-core/pull/16495
--
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: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org