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