jdaugherty commented on code in PR #16494:
URL: https://github.com/apache/grails-core/pull/16494#discussion_r4175699202
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -735,6 +747,88 @@ public void buildIndex() {
}
}
+ /**
+ * Runs the same build as {@link #buildIndex()} on this connection's
background index build thread, whatever
+ * {@code grails.mongodb.buildIndexesAsync} says, and returns at once with
a handle on its outcome.
+ *
+ * <p>The future completes with the build's {@link IndexBuildResult} once
every declaration has been applied,
+ * including when some of them failed: those are counted in {@link
IndexBuildResult#failures()} and logged
+ * as they happen. It completes exceptionally with the exception that
stopped the build partway, such as a
+ * lost connection or a write concern the server could not satisfy.
Created and already-present indexes are always told apart, whatever the log
level.
+ *
+ * <p>Builds on a connection run one at a time, so a build requested while
another is running waits for it,
+ * including one running on a caller's thread with {@link #buildIndex()}.
+ * Cancelling the future does not stop the build. If the datastore is
stopped or closed, the future completes
+ * exceptionally at once, and a build that stopping or closing the
datastore cuts short completes
+ * exceptionally too; a restart runs the cut-short build again, without a
future. Each named connection
+ * builds its own domain classes: call this on {@link
#getDatastoreForConnection(String)} for those.
+ *
+ * @return the outcome of the build
+ */
+ public CompletableFuture<IndexBuildResult> buildIndexAsync() {
+ CompletableFuture<IndexBuildResult> result = new CompletableFuture<>();
+ if (closed || !submitIndexBuild(result)) {
+ result.completeExceptionally(new IllegalStateException("The index
build for connection [" +
+ connectionName() + "] was not started: the datastore is "
+ (closed ? "closed" : "stopped") + "."));
+ }
+ return result;
+ }
+
+ /**
+ * Hands a build to this connection's background index build thread.
+ *
+ * @param result the future to complete with the build's outcome, or
{@code null} if nothing is waiting on it
+ * @return false if the executor has been shut down, by {@link #stop()} or
{@link #close()}
+ */
+ private boolean submitIndexBuild(CompletableFuture<IndexBuildResult>
result) {
+ ExecutorService executor = this.indexBuildExecutor;
+ if (executor.isShutdown()) {
+ return false;
+ }
+ try {
+ executor.execute(new IndexBuildTask(result, () -> {
+ // The first thing the build does: said only of a build that
is under way, and ahead of
+ // everything it logs, which it would not be if the submitting
thread said it.
+ LOG.info("Building the indexes declared by the domain classes
for connection [{}] on a " +
+ "background thread. Startup does not wait for them, so
a query issued before its index " +
Review Comment:
Non-blocking. This line now goes out for every `buildIndexAsync()` too, and
the `BootStrap` recipe in the guide waits on exactly that future, so in the
documented case startup does wait and the message says it does not. Either keep
this sentence for builds with no waiting future, or log something neutral when
`result != null`, such as "Building the indexes declared by the domain classes
for connection [{}] on a background thread; the caller is waiting for the
result."
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -902,6 +1023,236 @@ private void buildDeclaredIndexes(IndexBuildSummary
summary) {
}
}
+ /**
+ * The entities whose declared indexes this datastore builds: those mapped
to its connection. Every connection
+ * shares one mapping context, so an entity mapped only to another
connection is in it too.
+ */
+ private List<PersistentEntity> indexedEntities() {
+ String connection = connectionName();
+ List<PersistentEntity> entities = new ArrayList<>();
+ for (PersistentEntity entity :
this.mappingContext.getPersistentEntities()) {
+ // Only create Mongo templates for entities that are mapped with
Mongo
+ if (!entity.isExternal() &&
+ !(entity.isMultiTenant() && multiTenancyMode ==
MultiTenancySettings.MultiTenancyMode.SCHEMA) &&
+ ConnectionSourcesSupport.usesConnectionSource(entity,
connection)) {
+ entities.add(entity);
+ }
+ }
+ return entities;
+ }
+
+ /**
+ * The collections whose declared indexes this datastore builds, each with
the declarations of every class
+ * mapped to it: every class in an inheritance hierarchy maps to its
root's collection, and several classes
+ * can name the same one.
+ */
+ private Map<MongoNamespace, CollectionDeclarations>
declarationsByCollection() {
+ Map<MongoNamespace, CollectionDeclarations> collections = new
LinkedHashMap<>();
+ for (PersistentEntity entity : indexedEntities()) {
+ com.mongodb.client.MongoCollection<Document> collection =
getCollection(entity);
+ CollectionDeclarations declarations =
collections.computeIfAbsent(collection.getNamespace(),
+ namespace -> new CollectionDeclarations(collection, new
ArrayList<>()));
+ for (IndexDeclaration declaration : declaredIndexes(entity)) {
+ declarations.declarations().add(new EntityDeclaration(entity,
declaration));
+ }
+ }
+ return collections;
+ }
+
+ private record
CollectionDeclarations(com.mongodb.client.MongoCollection<Document> collection,
+ List<EntityDeclaration>
declarations) {
+ }
+
+ private record EntityDeclaration(PersistentEntity entity, IndexDeclaration
declaration) {
+ }
+
+ /**
+ * Lists the indexes the domain classes declare that their collections do
not have, on the collections
+ * whose declared indexes {@link #buildIndex()} builds for this
datastore's connection.
+ *
+ * <p>A declared index is present when its collection has an index on the
same key pattern, the same fields
+ * in the same order, whatever its name and options: an option that
differs is the build's to reconcile.
+ * A text index is present when one indexes the same text fields, in any
order, with the same keys before and
+ * after them, whatever its weights. Keys that several classes mapped to
one collection declare are reported
+ * once. Nothing is changed:
+ * {@link #buildIndex()} or {@link #buildIndexAsync()} creates them. Each
named connection has its own
+ * domain classes, so call this on {@link
#getDatastoreForConnection(String)} for those.
+ *
+ * @return the missing indexes, collection by collection, in the order the
domain classes declare them
+ */
+ public List<MissingIndex> findMissingIndexes() {
+ List<MissingIndex> missing = new ArrayList<>();
+ for (Map.Entry<MongoNamespace, CollectionDeclarations> entry :
declarationsByCollection().entrySet()) {
+ MongoNamespace namespace = entry.getKey();
+ List<Document> existing =
entry.getValue().collection().listIndexes().into(new ArrayList<>());
Review Comment:
Nit, non-blocking. `declarationsByCollection()` includes every mapped
collection, with or without declarations, so this lists the indexes of a
collection that declares nothing and can report nothing missing. Skipping an
entry whose `declarations()` is empty saves a `listIndexes` per such collection
on a method the guide suggests calling at startup. `findUndeclaredIndexes()`
needs the listing either way.
##########
grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/MongoDatastore.java:
##########
@@ -693,7 +717,9 @@ public ConnectionSources<MongoClient,
MongoConnectionSourceSettings> getConnecti
* <p>Each index is created by a command that the server answers only once
the index has been built,
* so this blocks the calling thread for as long as MongoDB takes to build
every declared index. With
* {@code grails.mongodb.buildIndexesAsync} enabled the work goes to a
background thread and this
- * returns immediately instead.
+ * returns immediately instead. {@link #buildIndexAsync()} runs it in the
background whatever the setting,
+ * and reports the outcome to its caller. Builds on a connection run one
at a time, wherever they run: a
Review Comment:
Non-blocking. One path is still outside both new rules.
`persistentEntityAdded` (line 2291) calls `initializeIndices(entity)` on the
registering thread without `indexBuildLock`, so a domain class registered while
a background build is running is the one remaining case where two builds touch
a collection at once. It also skips the `usesConnectionSource` check
`indexedEntities()` applies, so an entity registered at runtime and mapped only
to a named connection is indexed in the default connection's database. Taking
the lock there would make the registering thread wait out a running build, so
if that is not wanted, qualify this sentence and the matching one in
`buildIndexAsync()`'s Javadoc. The connection check is a one-line guard either
way.
--
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]
