Skip to content

Approve RavenDB index definitions to catch changes that trigger a rebuild - #6013

Open
ramonsmits wants to merge 2 commits into
masterfrom
index-definition-approvals
Open

ramonsmits wants to merge 2 commits into
masterfrom
index-definition-approvals

Conversation

@ramonsmits

@ramonsmits ramonsmits commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

A change to a RavenDB index definition makes RavenDB build a side-by-side replacement of the index at the next start-up. On a large audit database the rebuild takes days, and an index an operator locked after the Lucene migration does not receive the change until it is unlocked. 6.20.0 changed the audit MessagesViewIndex and MessagesViewIndexWithFullTextSearch definitions without a note in the upgrade guide, so the rebuild surprised operators who unlocked the index on 6.21.2.

This adds an approval test per RavenDB persister that renders the definition of every index the persister deploys. The rendered text contains the parts RavenDB compares when it decides to replace an index: maps, reduce, fields, configuration, additional sources and assemblies, output collection settings, lock mode, priority, state and deployment mode. The test discovers indexes the same way IndexDeployment.CreateIndexesAsync(Assembly, ...) does, and runs without a RavenDB server.

The document conventions are part of the deployed definition: with SaveEnumsAsIntegers an enum literal in a map renders as a number. The conventions now come from one factory, DocumentStoreConventions.Create() in ServiceControl.RavenDB, used by the embedded and external document stores and by the renderer, so the approved text is what the instances deploy and a change to the conventions fails the test.

  • ServiceControl.RavenDB/DocumentStoreConventions.cs is the shared factory
  • TestHelper/IndexDefinitionText.cs renders the definitions, shared by both test projects
  • IndexDefinitionApprovals in ServiceControl.Persistence.Tests.RavenDB and ServiceControl.Audit.Persistence.Tests.RavenDB, with the approved files for the current definitions
  • docs/testing.md states what to do when the test fails: record the changed indexes in the upgrade guide before approving the new definition

mauroservienti
mauroservienti previously approved these changes Oct 16, 2026
@mauroservienti
mauroservienti dismissed their stale review October 16, 2026 13:59

I'm still not sure this is enough

Comment thread src/TestHelper/IndexDefinitionText.cs
@mauroservienti

Copy link
Copy Markdown
Member

Today, getting past this test only takes copying the .received file over the .approved one. A few ideas to make an index change harder to slip through. They could go in this PR or a follow-up:

1. Record intent with each approved definition

Keep a manifest next to the approved files with one entry per index: a hash of the rendered definition, the version in which it last changed, and the upgrade-guide link. For example:

{
  "MessagesViewIndex": { "hash": "…", "changedIn": "6.20.0", "upgradeGuide": "https://docs.particular.net/…" }
}

The test fails if a hash changes but its entry is not updated. Re-approving then means stating which release changes the index and where operators can read about it. The manifest also gives release notes a reliable list of index changes.

2. Flag index changes on pull requests

Add a workflow that triggers on PRs touching **/ApprovalFiles/IndexDefinitionApprovals.*, src/*Persistence.RavenDB/**/Indexes/**, or src/ServiceControl.RavenDB/IndexDeployment.cs. It:

  • adds an index-rebuild label
  • posts a checklist comment: upgrade guide entry (docs PR link), note for locked indexes, expected impact on large databases
  • keeps a check red until a maintainer adds an acknowledgement label

Making that check required turns it into a gate rather than a reminder.

3. Check at release time

In release.yml, compare the approved index files between the previous tag and the commit being released. If they differ, fail the release, or require an explicit confirmation, unless the upgrade guide covers it. This catches several PRs that together change an index, which a per-PR check can't see. It is the check that would have caught 6.20.0.

For the operator side, I opened #6015: warn at start-up, using IndexHasChangedOperation, before a definition change triggers a rebuild, and flag locked indexes that will not receive it.

@ramonsmits
ramonsmits force-pushed the index-definition-approvals branch from 03977d8 to 111d718 Compare October 20, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants