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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
