Skip to content

Add grails.mongodb.buildIndexes and buildIndexesAsync for MongoDB index creation on startup - #16208

Merged
jdaugherty merged 37 commits into
apache:8.0.xfrom
codeconsole:feature/mongodb-skip-index-build-8.0.x
Sep 22, 2026
Merged

jdaugherty merged 37 commits into
apache:8.0.xfrom
codeconsole:feature/mongodb-skip-index-build-8.0.x

Conversation

@codeconsole

@codeconsole codeconsole commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

Two settings for the MongoDB index build that runs when the datastore starts, plus the reporting to make either decision an informed one.

All of these are runtime configuration, set in grails-app/conf/application.yml (or application.groovy) — nothing in build.gradle.

grails.mongodb.buildIndexes

grails:
    mongodb:
        buildIndexes: false

GORM creates and reconciles every index declared in a mapping block each time the datastore starts, with no way to switch it off. That is unwanted when deploying against live data whose indexes are managed separately, by a DBA or a migration step: the deployment builds indexes against production collections and reconciles the index set the running application depends on.

With the setting off, GORM issues no createIndex or collMod of its own for any domain class, and the indexes on the server are left exactly as they are. Persistence and querying are unaffected and keep using whichever indexes exist. It also covers domain classes registered after startup, and resolves per connection like every other connection setting, so index building can be switched off globally and left on for one connection:

grails:
    mongodb:
        buildIndexes: false
        connections:
            reporting:
                url: mongodb://localhost/reporting
                buildIndexes: true

The setting governs only what GORM does by itself. The application can still build the declared indexes when it chooses — once a deployment has been verified, from an administrative action, or on a schedule — by calling buildIndex() on the MongoDatastore bean:

class IndexMaintenanceService {

    MongoDatastore mongoDatastore

    void buildIndexes() {
        mongoDatastore.buildIndex()
    }
}

That runs the same build as startup, with the same summary, and honours buildIndexesAsync. A named connection builds through its own datastore: mongoDatastore.getDatastoreForConnection('reporting').buildIndex(). A collection never built with the setting on or by such a call has no declared indexes, so this is for environments where the indexes are already in place, are applied by other means, or are built on demand. It governs only what GORM derives from mapping blocks; an explicit createIndex in application code is unaffected.

grails.mongodb.buildIndexesAsync

grails:
    mongodb:
        buildIndexesAsync: true

MongoDB answers a createIndex only once the index has been built, so by default startup waits for every declared index in turn. Measured against MongoDB 7.0 with 200k documents: 172ms to create one index on the calling thread, 1.2ms to re-issue the same declaration. background: true has been ignored since MongoDB 4.2 and costs the same.

With this enabled index builds — the startup one, and any the application starts with buildIndex() — run on one daemon thread per connection, named gorm-mongo-index-build-<connection>-<n>, and startup continues without waiting. Indexes are still built one at a time per connection. Different connections can build concurrently, including against the same server. Each worker is released after one idle second; another explicit build starts a new worker. Closing the datastore interrupts the builds for every connection before closing the clients, and a later buildIndex() logs a warning rather than throwing. Stopping it for a checkpoint (CRaC) interrupts a build still running instead of letting it fail against the closed client, and restarting it runs that build again.

Worth planning for:

  • A query issued before its index has been built is served without it — correctly, but unindexed. A unique index likewise constrains nothing until the build finishes.
  • Startup no longer waits, so a failed index build can no longer fail startup. It is logged at error level and the application runs without that index. The default synchronous build still propagates the exception.
  • A domain class registered after startup is indexed on the thread registering it, which keeps the tenant context that path can depend on.

Index build reporting

A successful build previously logged nothing at all — only conflicts, TTL updates and failures — so there was no way to tell how much of startup went on indexes, and with the async build no signal that it had finished. It now logs one summary:

Index build for database [myDb] finished in 412ms: 2 created, 5 already present, from 3 domain class(es)

The created/already-present split is what makes the elapsed time interpretable: a restart that changed no mappings reports everything as already present and costs milliseconds, so a summary reporting indexes created is the one that accounts for a slow start. An index recreateOnConflict dropped and built again costs as much as a new one and is counted apart as recreated. A build with failures is logged at WARN and reports how many, and a build that stops partway reports how far it got:

Index build for database [myDb] did not finish, stopping after 812ms at 1 of 3 domain class(es): 1 created, 0 already present

Each index also logs its own elapsed time at DEBUG under org.grails.datastore.mapping.mongo, which is how to find the single slow index behind a slow summary.

createIndexes reports numIndexesBefore / numIndexesAfter, but the driver's createIndex helper discards the response and returns only the index name. Rather than hand-roll the command — the IndexOptions to index-specification mapping is not worth reimplementing — the build lists the indexes a collection already has. That listing is lazy, so an entity declaring no indexes costs no round trip, and it is only issued when INFO is enabled for org.grails.datastore.mapping.mongo: with the summary not being logged, the build skips it. A conflict lists again rather than trusting that snapshot, since the index the server reports as conflicting may have been created since. Successful creation, TTL updates and recreation update the snapshot so later declarations use the current index name and TTL. A duplicate declaration counts as already present after the first creates it. If the initial listing cannot be read, the build still runs and the summary falls back to reporting how many declarations were applied. The domain-class count includes classes that declare no indexes.

Settings were ignored when the application supplies its own MongoClient

Found while testing the above. The constructors taking a MongoClient (or a MongoClientSettings.Builder) built the default connection source from a bare MongoConnectionSourceSettings, so every setting describing how the datastore behaves — the grails.mongodb block (multiTenancy, stateless, transactional, engine, flush mode, decimalType, and now buildIndexes) and the grails.gorm block it falls back to — was silently left at its default. Only databaseName survived, because it is set explicitly from the mapping context.

That path is not obscure: Spring Boot's MongoDB auto-configuration contributes a MongoClient bean and MongoDbGormAutoConfiguration hands that client to GORM whenever one is present, so a Spring Boot application configuring GORM through grails.mongodb was being ignored. The settings are now bound from the configuration on that path too; the connection details in them go unused, as the client arrives already connected.

This means configuration an application already has starts taking effect: failOnError turns an invalid save() into a ValidationException, the default mapping and constraints are applied, autoFlush / markDirty / flushMode are honoured, and configured multi-tenancy replaces the NONE mode the supplied-client path was silently running in. The multi-tenancy part is security-relevant, and it needs one step that is easy to miss: a TenantResolver bean is injected into the connection source factory, and there is no factory on this path, so the resolver has to be named through tenantResolverClass or the mode is active with NoTenantResolver throwing TenantNotFoundException on every tenant-scoped operation.

The full list, and what to check before upgrading, is in the guide under Upgrade Notes → "Configuration Is Now Applied to an Externally-Supplied Client", beside the existing note on the client lifecycle.

Notes

  • Documented in grails-data-mongodb/docs under Querying Indexing and Advanced Configuration, with the 8.0 release notes updated. Those pages showed configuration only in the application.groovy DSL, so they now show the application.yml form as well, and note that these settings are read by name — they are not relaxed-bound like Spring Boot's own properties, so a kebab-case spelling such as build-indexes is ignored and leaves the default in place.
  • The mongo core tests now run against logback rather than the no-op SLF4J binding, as grails-data-neo4j/core already does, so a test can assert on what was logged. The root logger is pinned to WARN by a logback-test.xml, and the build output is unchanged.

…rtup

GORM created and reconciled every index declared in a domain class mapping
block each time the datastore started, with no way to switch it off. That is
unwanted when deploying against live data whose indexes are managed
separately: the deployment builds indexes against production collections and
reconciles the index set the running application depends on.

Setting grails.mongodb.buildIndexes = false leaves the server's indexes
exactly as they are - no createIndex or collMod command is issued for any
domain class, including for domain classes registered after startup.
Persistence and querying are unaffected and continue to use whichever
indexes already exist.

The setting is resolved per connection like every other connection setting,
so index building can be switched off globally and left on for an individual
connection.
The constructors that take a MongoClient (or a MongoClientSettings.Builder)
built the default connection source from a bare MongoConnectionSourceSettings,
so every setting under grails.mongodb that describes how the datastore behaves
- stateless, transactional, engine, flush mode, decimalType, buildIndexes -
was silently left at its default. Only databaseName survived, because it is set
explicitly from the mapping context.

That path is not obscure: Spring Boot's MongoDB auto-configuration contributes
a MongoClient bean, and MongoDbGormAutoConfiguration hands that client to GORM
whenever it is present, so a Spring Boot application configuring GORM through
grails.mongodb was being ignored.

The settings are now bound from the configuration on this path too. The
connection details in them go unused, as the client is supplied already
connected.
… thread

MongoDB answers a createIndex command only once the index has been built, and
the datastore builds every declared index from its constructor, so an
application waits at startup for all of them in turn. On an empty collection
that is instant; on a large existing collection it is minutes of a deployment
spent waiting.

Setting grails.mongodb.buildIndexesAsync = true hands the startup build to a
single daemon thread per connection, named gorm-mongo-index-build-<connection>,
and returns immediately. Indexes are still built one at a time rather than all
at once against the server.

Two consequences are documented rather than hidden: a query issued before its
index exists is served without it (a unique index likewise constrains nothing
until the build completes), and a build failure can no longer fail startup, so
it is logged at error level instead. The setting covers the startup build only
- a domain class registered later is still indexed on the registering thread,
which keeps the tenant context that path can depend on.
A successful index build said nothing at all: only conflicts, TTL updates and
failures were logged, so there was no way to tell how much of startup went on
building indexes, and with buildIndexesAsync no signal that the background
build had finished.

The build now logs one summary when it completes:

    Applied 7 index declaration(s) from 3 domain class(es) to database [myDb] in 412ms

The count is declarations applied rather than indexes built, because
createIndex is idempotent and the server accepts one that already exists
without doing any work - claiming otherwise would be wrong on every restart
after the first. A build with failures is logged at warn and reports how many.
Each index also logs its own elapsed time at debug, which is how to find the
one slow index behind a slow summary.

The tests now run against logback rather than the no-op SLF4J binding, as the
sibling data modules do, so that a test can assert on what was logged; the
root logger is quietened to WARN so this does not add noise to the build.
The previous summary counted declarations applied and said, in as many words,
that it could not tell whether any of them did work. That was wrong about the
server: createIndexes answers with numIndexesBefore, numIndexesAfter and a
"note: all indexes already exist", and it is the driver's createIndex helper
that discards the response and returns only the index name.

Rather than hand-roll the command to read that response - the mapping from
IndexOptions to an index specification is not worth reimplementing - the build
lists the indexes a collection already has, once per collection, and reports:

    Index build for database [myDb] finished in 412ms: 2 created, 5 already present, from 3 domain class(es)

That split is what makes the elapsed time mean something. Measured against
MongoDB 7.0 with 200k documents: creating an index took 172ms on the calling
thread, re-issuing the same declaration 1.2ms. A restart that changed no
mappings reports everything as already present, so a summary reporting created
indexes is the one that accounts for a slow start.

The listing is lazy, so an entity declaring no indexes costs no round trip, and
it replaces the listing the conflict path used to make for itself. If it cannot
be read the build still runs and the summary falls back to reporting how many
declarations were applied.
@codecov

codecov Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.07547% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.3454%. Comparing base (8c3851b) to head (91a7eb4).

Files with missing lines Patch % Lines
...grails/datastore/mapping/mongo/MongoDatastore.java 92.0755% 13 Missing and 8 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##                8.0.x     #16208        +/-   ##
==================================================
+ Coverage     56.2429%   56.3454%   +0.1025%     
- Complexity      21795      21863        +68     
==================================================
  Files            2127       2127                
  Lines          103229     103460       +231     
  Branches        18489      18536        +47     
==================================================
+ Hits            58059      58295       +236     
+ Misses          36890      36884         -6     
- Partials         8280       8281         +1     
Files with missing lines Coverage Δ
...tions/AbstractMongoConnectionSourceSettings.groovy 70.0000% <ø> (ø)
...grails/datastore/mapping/mongo/MongoDatastore.java 78.8051% <92.0755%> (+9.9492%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…n.groovy

The configuration examples were written only in the application.groovy DSL,
which reads as though the Groovy form were the only one; application.yml is
what a generated Grails application actually ships with.

Both forms are now shown, and the settings are noted as being read by name:
they are not relaxed-bound the way Spring Boot's own properties are, so a
kebab-case spelling is ignored and silently leaves the default in place.
@codeconsole codeconsole added this to the grails:8.0.0-RC1 milestone Sep 1, 2026
Codecov put the patch at 75%, and what it was missing was every path where
something goes wrong - which is where behaviour that has been claimed in the
documentation most needs pinning. Four of them are now covered:

- a declaration the server refuses, and one whose conflict cannot be
  reconciled, are counted and reported: the build carries on, the summary is
  raised to WARN and says how many failed
- a background build that fails logs the failure at error level, since nothing
  waits on that thread to notice it
- a background build interrupted by shutdown is logged as the shutdown it is,
  not as a failure
- a role that may create an index but not list the existing ones still gets its
  indexes, with the summary falling back to how many declarations it applied

The last two need conditions a server will not produce on demand, so the driver
is wrapped in a proxy that makes one operation fail or block. Only that one
method is intercepted; everything else is the real driver talking to the real
server, and the datastore under test is the real one built through its public
constructor.

Patch coverage is now 97.8%. What remains is two error branches in conflict
reconciliation that predate this change and are not reachable without a server
that contradicts itself.
@codeconsole
codeconsole dismissed stale reviews from matrei and jdaugherty September 21, 2026 17:47

completed

@matrei matrei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review round 5

Head: 848282548a (codeconsole:feature/mongodb-skip-index-build-8.0.x) · Base: 8.0.x. This round covers the seven commits since 4ebb343fce: 423ac93345, 4a4733ac09, 8a7e1ff4c4, 4c074122eb, f4bb9466e6, fd13f1a3d6 and 848282548a. The merge-base is still 3dc36325dc, but 8.0.x has moved 81 commits since (now 497ec8d588), several of them in grails-data-mongodb. A trial merge of origin/8.0.x into the head is conflict-free, and that merged tree is what I ran.

Run locally (Docker, MongoDB via Testcontainers), on the head merged with origin/8.0.x: :grails-data-mongodb-core:cleanTest :grails-data-mongodb-core:check --no-build-cache --continue gives 790 tests, 0 failures, 0 errors, 45 skipped across 224 freshly written result files (the 785 reported on the PR plus 5 that came in with the base), with checkstyleMain and codenarcMain executed and clean. All eight new or extended specs ran: BuildIndexesLifecycleSpec (2), BuildIndexesStaleSnapshotSpec (1), BuildIndexesRecreateSummarySpec (1), BuildIndexesUnfinishedSummarySpec (2), BuildIndexesClassificationCostSpec (1), BuildIndexesSharedDeclarationSpec (1), SuppliedMongoClientSettingsSpec (14), MongoConnectionSourceSettingsSpec (6). I did not run the docs build or the functional tests this round.

CI: the one red job, "Build Grails-Core (Windows JDK 25 shard 0)", failed in :build-logic:compileGroovy with Could not get resource 'https://repo.maven.apache.org/maven2/org/ow2/asm/asm/9.10.1/asm-9.10.1.pom'. That is a download failure, not something this PR causes; a re-run should clear it. The rest of the matrix was still pending when I looked.

Verdict: everything from round 4 is addressed the way it was triaged, each fix has a test that exercises it through the public API, and I found no new defect in the lifecycle code. Two small things remain, neither blocking. Approving again from my side; comments 10 and 11 remain @jdaugherty's to accept.

Findings

1. The two new settings are missing from the plugin's configuration metadata (grails-data-mongodb/grails-plugin/src/main/resources/META-INF/spring-configuration-metadata.json, low)

The plugin ships hand-written metadata for the grails.mongodb.* settings (url, databaseName, stateless, decimalType, codecs, ...), and it has no entry for grails.mongodb.buildIndexes or grails.mongodb.buildIndexesAsync. I missed this in the earlier rounds; the file has been there since February. Two boolean entries, defaults true and false, with the one-line descriptions from advancedConfig.adoc.

Note that 8.0.x has since renamed the file to additional-spring-configuration-metadata.json (d12545ae42), so this is easiest done after merging the base in.

2. The deferral of a build requested while stopped has no test (MongoDatastore.java:656-662, low)

The reply on the buildIndex() thread says both the after-close() warning and the deferral to start() are in BuildIndexesLifecycleSpec. The spec has the first, and the stop()/start() feature covers a build that stop() cut short, but nothing calls buildIndex() between stop() and start(). A third feature would do: let the first build finish, stop(), buildIndex(), assert no exception and the INFO line, drop the index through the real client, start(), wait for the index.

A related detail on the same lines: close() does not clear running, so the branch keys on "stopped" versus "not stopped", and a datastore that was stopped and then closed answers buildIndex() with "it will run when the datastore is restarted". Harmless, and not worth a flag of its own; mentioning it only so the wording of the WARN is not read as a guarantee.

Notes, no action needed

  • Merge 8.0.x before this lands. It is clean today, but 81 commits including AbstractMongoSession, MongoTransaction and the metadata rename above are not in what CI is testing.
  • stop() does not wait for the interrupted worker. shutdownNow() returns immediately, so a worker whose initializeIndices override swallows the interrupt could in principle still be alive when start() submits the new build on the new executor. In the base class the worker dies at its next driver call (interrupted, or the closed client), a checkpoint and restore sit between the two calls, and index creation is idempotent, so I would leave it.
  • The spurious indexBuildPending. shutDownIndexBuild() reads the in-flight count before shutdownNow(), so a build finishing in between is reported as cut short and runs again on start(). That costs one createIndex per declared index against indexes that exist. Fine.
  • "at 1 of 3 domain class(es)" counts completed classes (summary.entities++ now follows the hook), so the class that failed is the next one. The guide's example reads correctly with that meaning.

Verified as correct

  • Comment 1, shared compound declaration (4a4733ac09): the attributes are removed from a LinkedHashMap copy, the declaration is untouched, key order is preserved for the index definition. BuildIndexesSharedDeclarationSpec asserts unique on both connections with the default synchronous build.
  • Comment 2, supplied mapping context's database (8a7e1ff4c4): settings.url(null) clears the one connectionString field that both grails.mongodb.url and grails.mongodb.connectionString bind to, so getDatabase() falls back to databaseName. Nothing else on the supplied-client path reads the connection string: the factory is only asked to create a client from start(), which returns early when the client is not owned.
  • Comment 3, recreated count: Reconciliation separates UPDATED (counted as already present) from RECREATED; describe() prints the recreated count in both the classified and the unclassified form and the debug "no indexes declared" branch now also requires recreated == 0. contains() stays on findIndexByKeyPattern, for the text-index reason given in round 4.
  • Comment 4, re-listing on conflict: reconcileIndexConflict calls refresh(), which also replaces the cached listing (and nulls it on failure, so record() does not write into a stale list). BuildIndexesStaleSnapshotSpec pins two listings and the in-place TTL update.
  • Comments 5 and 6, lifecycle: stop() interrupts before closing the client and returns early, as before, for a client it does not own, so a supplied client's build is not touched. start() replaces the executor before setting running and only then runs the pending build. The worker captures its own executor, so abandoned is judged against the executor that was shut down, not the replacement. The in-flight count is corrected for tasks shutdownNow() removed from the queue, and for a RejectedExecutionException between the check and the submit. The field is volatile and read once per method.
  • Comment 7, unfinished summary: a synchronous build logs the WARN and rethrows the original exception unchanged; a background build logs the error first and the partial summary after it, at DEBUG when abandoned. Both modes are in BuildIndexesUnfinishedSummarySpec.
  • Comment 8: the Javadoc on initializeIndices(PersistentEntity) says what an override must not rely on, and CheckpointedDatastore in the lifecycle spec is written to that rule (static latches).
  • Comment 9, ConcurrentHashMap (f4bb9466e6): I checked the four lookups for null keys, which ConcurrentHashMap.get rejects and LinkedHashMap.get did not. Every call site dereferences the key first (connectionName.equals(...), tenantId.toString()) or has already passed it to connectionSources.getConnectionSource(qualifier), so no caller that worked before now throws. close() is the only iteration, so nothing depended on insertion order. The field's declared type is still Map, so subclasses are unaffected.
  • Comment 11, the isInfoEnabled() gate: contains() short-circuits when the summary is not classified, a hook call outside a build uses an unclassified summary (so registering an entity at runtime no longer lists either), and the summary then prints "N index declaration(s) applied". BuildIndexesClassificationCostSpec counts listIndexes commands with a CommandListener: 0 at WARN, 1 at INFO. The guide says which logger controls it.
  • Comment 12: CustomizableThreadFactory with the prefix gorm-mongo-index-build-<connection>-, daemon; the guide and the two specs that assert the thread name (BuildIndexesAsyncSpec, BuildIndexesPerConnectionSpec) match on the new name.
  • Comment 13: MongoConnectionSourceSettingsSpec now has the feature through a StandardEnvironment with ConfigurationPropertySources.attach, and it passes, so the NOTE is pinned against the environment applications run with.
  • Comment 14 (fd13f1a3d6): CapturedLog backs the appender with a CopyOnWriteArrayList, hands out copies, and restores the logger's previous level and detaches on close(), so the specs leave no logger state behind for the next spec in the fork.
  • Round 3, finding 1 (4c074122eb): SuppliedMongoClientSettingsSpec pins both directions, the configured tenantResolverClass and NoTenantResolver without it.

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 4. All fourteen threads from the last round are answered — eleven fixed and verified against the branch, three argued rather than changed, and the three arguments hold. Two of my findings were simply wrong and the replies correct them: the text-index contains() (a strict comparison would report every unchanged text index as created on every restart) and the relaxed-binding note (grails.mongodb.databaseName is not a valid ConfigurationPropertyName, so Boot's attached source never answers it). I have resolved those threads and left one open, on stop()/start() and the per-connection children.

One blocking item, on SuppliedMongoClientSettingsSpec: this branch breaks :grails-data-mongodb-core:test under JDK 25 — 7 failures on both MongoDB 7.0 and 8.0, where CI was green on 4ebb343. It reproduces locally (JDK 25 red, JDK 21 clean, the same split CI shows) and it is not an assertion failure: mongod aborts on an invariant in op_observer.cpp, taking down the container that the whole Gradle fork shares, which is why five MongoStaticApiMultiTenancySpec features fail alongside. Details and the two candidate one-line fixes are on that thread.

The rest are follow-ons from this round's fixes rather than fresh ground. The one I would not ship without is sameKeyPattern comparing key sets rather than key order: it was unreachable while it only ran after an IndexOptionsConflict, but the new classification calls it for every declaration, so a genuinely new compound index declared in a different key order is reported as already present and displaces the real index from the snapshot.

The feature cleaned up with CompanyB.eachTenant { CompanyB.DB.drop() }. In
SCHEMA mode every database on the server is a tenant, MongoDB's own admin,
config and local included; admin and local refuse the drop and the catch
swallowed it, but config was dropped - on the test server the fork shares with
every other AutoStartedMongoSpec.

That takes config.image_collection with it. The next retryable findAndModify on
that server - GORM's native id generation, in any later specification - makes
mongod recreate the collection inside the write, which fails the invariant
'_times._recursionDepth == 1 || !opCtx->writesAreReplicated()' in
op_observer.cpp and aborts the server (exit 133). Every specification after it in
the fork then finds nothing listening.

It surfaced on this branch because the new specification classes changed how
CI spreads classes over its four forks, putting this spec ahead of
SuppliedMongoClientSettingsSpec in one of them: seven failures in the MongoDB
7.0, MongoDB 8.0 and coverage jobs, all against the same dead server. The crash
was caught with docker events and the container's own log.

The cleanup now drops only the two tenant databases the feature uses.
…etadata

The plugin ships hand-written metadata for its grails.mongodb settings, which
is what an IDE completes and documents from, and neither
grails.mongodb.buildIndexes nor grails.mongodb.buildIndexesAsync was in it.
Both now are, with their defaults, and both appear in the
spring-configuration-metadata.json the plugin jar is built with.
sameKeyPattern checked that two key patterns had the same keys and directions,
not that they came in the same order. To MongoDB {a: 1, b: 1} and {b: 1, a: 1}
are two indexes, and one cannot serve the other's sort.

That was harmless while the comparison only ran after the server reported an
IndexOptionsConflict, which it does only for the same key pattern. The build now
also uses it to tell a new index from an existing one, before createIndex, so a
compound index declared in a different order from one already on the server was
reported as already present while MongoDB created it - and recording it then
displaced the existing index from the snapshot, so a later declaration of that
one was reported as created. The keys are now compared position by position; the
text-index special case, checked first, is unchanged.
…osure

The closure configured nothing; every other entry in the file uses one to
configure something and keeps its note alongside.
…ting

Follows up the review of the previous round:

- A per-connection child starts its index build only once it is registered, and
  a connection added at runtime starts it only if the datastore is not closing.
  close() sets that flag before walking the children, so it either finds a
  child and shuts its build down or the build never starts; before, a child
  added during close() could run on unseen against a closed client and log its
  shutdown as a failure.

- stop() and start() cover every connection, as close() does: each child's
  build is interrupted before the client goes and resumed afterwards.

- Whether a build was cut short is now exact. A build interrupted by stop()
  records it as it exits, so one that finishes in the meantime is not re-run;
  a queued build is taken from shutdownNow(). start() waits for the interrupted
  worker to exit before deciding, rather than racing it.

- buildIndex() tells closed from stopped with a flag close() sets, so a
  datastore stopped and then closed warns instead of promising a restart.

- runIndexBuild catches Exception rather than Throwable, so an Error propagates
  as it is; an InterruptedException restores the interrupt flag; and a build on
  the calling thread rethrows the original exception, checked or not, instead
  of wrapping it in IllegalStateException.

- The listing is cached per collection for the whole build rather than per
  entity, so classes sharing a collection list it once and see each other's
  indexes.

- An unreadable listing is reported with its own reason again, as on 8.0.x,
  rather than with the conflict's message.

- The per-index DEBUG line says Applied when nothing classified the index,
  instead of claiming every index outside a build was created.
@codeconsole

Copy link
Copy Markdown
Contributor Author

Addressed round 5 (@matrei) and round 4 (@jdaugherty); there is a reply on each of the new threads and on the one left open.

The CI failures. mongod was crashing on the op_observer.cpp invariant because SchemaBasedMultiTenancySpec dropped MongoDB's config database on the container its fork shares. In SCHEMA mode every database is a tenant, and its cleanup was eachTenant { DB.drop() }. The next retryable findAndModify then had to recreate config.image_collection inside the write. The cleanup now drops only its own two tenant databases (6fe3738). The details and the container log are on @jdaugherty's thread. The spec dates from 2020; the new specification classes on this branch only changed which specs share a fork.

@matrei, round 5:

  1. The configuration metadata has both settings (5cafdb2), and both appear in the spring-configuration-metadata.json of the built plugin jar. While in that file: grails.mongodb.decimalType is listed with a default of false, while AbstractMongoConnectionSourceSettings defaults it to true. That predates this PR, so I've left it.
  2. The deferral while stopped is now a feature in BuildIndexesLifecycleSpec. On the related detail: buildIndex() now decides between the warning and the deferral on a closed flag that close() sets, rather than on running, so a datastore that was stopped and then closed gets the warning.

From the notes: 8.0.x is merged (abc944e), start() now waits for the interrupted worker before deciding, and the spurious re-run is gone (the pending-flag thread).

@jdaugherty, round 4: every item is fixed with a test except the URL scrub, which I've argued on its thread. sameKeyPattern now compares in order (9a88f6e), and the rest is in d8adb08.

Run locally, on the head merged with 8.0.x, with -PmaxTestParallel=4 as CI uses and a watcher on docker events recording any mongod that exits without being stopped:

  • JDK 21, :grails-data-mongodb-core:cleanTest :grails-data-mongodb-core:check --no-build-cache --continue, twice: 799 tests, 0 failures, 0 errors, 45 skipped each time, with checkstyleMain and codenarcMain clean.
  • JDK 25, :grails-data-mongodb-core:cleanTest :grails-data-mongodb-core:test, twice: 799 tests, 0 failures, 0 errors, 45 skipped each time.
  • No mongod exited abnormally in any of the four runs.
  • LocalMongoUnitSpec passes both features; :grails-data-mongodb-docs:asciidoctor, root aggregateViolations and rat are clean.

@testlens-app

This comment has been minimized.

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at d8adb08. Eleven of the twelve open findings are fixed; I checked each against the code and ran :grails-data-mongodb-core:test twice on JDK 25, the configuration that gave me seven failures last round: 799 tests, 0 failures, 45 skipped, and no mongod exit either time.

What is left is in three threads — the resume path start() takes after a build it interrupted, the registration guard's synchronous half, and the connection details the supplied-client path drops — plus four new comments here. Let's discuss them and then decide if we can merge.

Comment thread grails-data-mongodb/docs/src/docs/asciidoc/querying/queryIndexes.adoc Outdated
Comment thread grails-data-mongodb/core/build.gradle Outdated
The upgrade note listed everything that starts applying to an externally
supplied client without saying that the connection details are the exception:
grails.mongodb.url is not applied, since the client is already connected. A
connection under grails.mongodb.connections without a url of its own therefore
falls back to the host, port, username and password settings - localhost:27017
unless they are set - rather than to grails.mongodb.url. The note now says so,
and that such a connection should declare its own url.
…ection

Follows up the review of the previous round:

- Classification is per collection. A listing that fails - first or re-listing
  after a conflict - leaves that collection's declarations unclassified instead
  of discarding the breakdown the other collections established, and the
  summary reports them as "applied without a listing" alongside it.

- A background build that fails while the datastore shuts down is reported as
  abandoned only when the failure is one the shutdown produces: an
  interruption, the driver refusing the closed client, or a socket closed
  under a call. Anything else - a duplicate key on a unique index - stays at
  ERROR and is not marked for re-running on restore.

- buildIndex() checks closed before either mode runs, so a build on the
  calling thread is refused after close() as a background one is, instead of
  running against the client being closed.

- start() no longer waits on an executor that was never shut down - a
  connection added while stopped - and so no longer starts a duplicate build
  of it; the wait for an interrupted build to exit is one second, not ten.

- Every line buildIndex() logs names its connection, the disabled message names
  the setting where an operator would look for it, and the background build is
  announced only once it has been submitted.

- codecRegistry is volatile, since the background build reads it while the
  @Autowired setters may still be replacing it.

FailingMongoClient can pass its interceptor the collection being called, so a
test can make one collection's listing fail and not another's.
@codeconsole

Copy link
Copy Markdown
Contributor Author

Addressed the seven open items; there is a reply on each thread.

  • Fixed: classification is now per collection, with the summary keeping the breakdown and reporting the rest as "applied without a listing"; a failure during shutdown counts as abandoned only if the shutdown explains it; the synchronous half of the registration guard; start() no longer waiting on, or re-running, a connection added while stopped, with the wait cut to one second; the connection name in every build log line, and the background announcement moved after the submission; and codecRegistry made volatile. All are in c6a64e8, each with a test.
  • Upgrade note: it now says the connection details are the exception, and that a named connection on the supplied-client path wants its own url (0ffc921).
  • CRaC: the three pre-existing gaps are filed as MongoDB CRaC checkpoint and restore cover only the default connection #16366, credited to @jdaugherty.

Run locally, on the head with -PmaxTestParallel=4 and the docker events watcher: JDK 21, :grails-data-mongodb-core:cleanTest :grails-data-mongodb-core:check --no-build-cache --continue, gives 804 tests, 0 failures, 0 errors, 45 skipped, with checkstyleMain and codenarcMain clean and no mongod exit. :grails-data-mongodb-docs:asciidoctor, root aggregateViolations and rat are clean. I did not repeat the JDK 25 runs this round; CI covers them.

@jdaugherty jdaugherty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix the one doc comment and then merge?

----
Index build for database [myDb] did not finish, stopping after 812ms at 1 of 3 domain class(es): 1 created, 0 already present
----

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph and the summary format above it are now one count short.

A collection whose listing fails no longer takes the whole build's breakdown with it — its declarations are counted apart, and the line gains a third token:

Index build for database [myDb] finished in 412ms: 2 created, 5 already present, 3 applied without a listing, from 3 domain class(es)

Nothing in the guide says what that is, and it is the one token an operator cannot guess from its wording: "applied without a listing" reads like work that was skipped, when it means the opposite — the declarations were applied, and only the created/already-present split for that one collection is missing, because listIndexes on it failed.

The sentence here is also narrower than the behaviour now is. The N index declaration(s) applied fallback is what you get when nothing was classified — the logger at WARN, or no collection readable at all — rather than specifically a build with failures. The new count is the partial case between the two, so both are worth a line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 91a7eb4. The section now covers all three forms the summary takes. The partial case has its own example line and says what it means: those declarations were applied, and only the created/already-present split is missing for a collection whose listIndexes failed. The N index declaration(s) applied form is tied to its actual condition, nothing classified (the logger at WARN, or no collection's indexes readable), with an example of the line you get at WARN.

grails.mongodb.buildIndexes = false made MongoDatastore.buildIndex() a no-op,
so an application that deploys without touching its indexes had no way to
build them later - after a deployment has been verified, from an
administrative action, or on a schedule - short of flipping the setting and
restarting.

The setting now governs only the builds GORM starts by itself: at startup, for
each named connection, for a connection added at runtime and for a domain class
registered later. An explicit buildIndex() always builds, refusing after close()
and deferring while stopped as before. It honours buildIndexesAsync, so the
index build executor now exists whenever builds are asynchronous rather than
only when GORM builds by itself; it holds no thread until a build is submitted.
Each named connection builds its own domain classes, through its own datastore.

The guide gains a "Building Indexes on Demand" section with the example, and the
configuration reference, release notes, setting Javadoc and configuration
metadata describe the setting as what GORM does by itself.
@codeconsole

Copy link
Copy Markdown
Contributor Author

One change in behaviour since the last round, in 7d927e1, so that it doesn't arrive unannounced.

grails.mongodb.buildIndexes = false made MongoDatastore.buildIndex() a no-op. So an application that deploys without touching its indexes had no way to build them afterwards — once the deployment is verified, from an admin action, or on a schedule — short of flipping the setting and restarting. That seemed the wrong reading of "off", and it is cheaper to settle before this ships than after.

The setting now governs only the builds GORM starts by itself: at startup, for each named connection, for a connection added at runtime, and for a domain class registered later. An explicit buildIndex() always builds. It still refuses after close() and defers while stopped. It honours buildIndexesAsync, so the executor exists whenever builds are asynchronous, not only when GORM builds by itself; it holds no thread until something is submitted. A named connection builds through its own datastore.

  • Tests: BuildIndexesOnDemandSpec covers an explicit build with the setting off, synchronously, on the background thread, and for a named connection, and checks that a stop()/start() still builds nothing by itself. The BuildIndexesDisabledSpec feature that pinned the old no-op is gone.
  • Docs: the guide has a new "Building Indexes on Demand" section with the example. The configuration reference, release notes, setting Javadoc and configuration metadata now describe the setting as what GORM does by itself.
  • Run locally on JDK 21 with -PmaxTestParallel=4: :grails-data-mongodb-core:check gives 807 tests, 0 failures, 45 skipped, with style clean and no mongod exit. The docs build, aggregateViolations and rat are clean.

…ery summary form

The announcement that a build is running on a background thread was logged
by the submitting thread after the task was handed over, so a fast build's
own lines could come before it. It is now the first thing the submitted
task logs. start() logs, on its own thread, when it resumes a build that
was pending while the datastore was stopped.

The guide now explains the "applied without a listing" count, and says
that the "index declaration(s) applied" form appears whenever nothing
could be classified, not only for a build with failures.
@codeconsole

Copy link
Copy Markdown
Contributor Author

@jdaugherty thanks, both taken in 91a7eb4:

  • Doc: "What the Index Build Reports" now explains applied without a listing and when the index declaration(s) applied form appears (replied in its thread).
  • Announcement order: the background build now logs its announcement as the first thing the submitted task does, so it comes from the build thread and always precedes the build's own lines. BuildIndexesAsyncSpec pins it: announcement and summary from the same thread, announcement first. Four lifecycle tests had relied on the announcement being logged on the calling thread as their synchronous sign that start() did or did not re-run a build. start() now logs Resuming the index build for connection [...] that was pending while the datastore was stopped. on its own thread when it does, and those tests assert on that instead. It also tells an operator why a build starts again after a restore.

The IllegalStateException breadth and the self-cause-only cycle guard are left as they are.

One thing to look at before merging: your approval was on c6a64e8, and 7d927e1 came after it. buildIndex() now runs when the application calls it even with buildIndexes: false; the setting turns off only the builds GORM starts by itself. It is documented under "Building Indexes on Demand" and covered by BuildIndexesOnDemandSpec.

@jdaugherty
jdaugherty merged commit 1fadd5c into apache:8.0.x Sep 22, 2026
89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants