Approve RavenDB index definitions to catch changes that trigger a rebuild

[ramonsmits]

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.

• 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

I'm still not sure this is enough

[mauroservienti]

The rendered definitions don't match what ServiceControl deploys. Both document stores are created with SaveEnumsAsIntegers = true (EmbeddedDatabase.Connect and RavenExternalPersistenceLifecycle), and IndexCreation assigns the store conventions to each index before calling CreateIndexDefinition(). Here the indexes have the default conventions, so enum comparisons are rendered as strings:

Approved: docs.FailedMessages.Where(doc => doc.Status == "Archived")
Deployed: docs.FailedMessages.Where(doc => doc.Status == 4)

When I set the runtime conventions locally, the test in ServiceControl.Persistence.Tests.RavenDB fails on that line. The audit approved file only matches because the audit indexes have no enum literals.

Because of this, a change to the conventions, for example someone flipping SaveEnumsAsIntegers, changes every deployed definition and rebuilds every index, and this test does not notice.

Suggested change

	Render(builder, index.CreateIndexDefinition());
	index.Conventions = new DocumentConventions { SaveEnumsAsIntegers = true };
	Render(builder, index.CreateIndexDefinition());

(plus using Raven.Client.Documents.Conventions;, and the primary instance approved file needs re-approving)

Better still: move the conventions into one shared factory in ServiceControl.RavenDB that both lifecycles and this helper use, so they can't drift apart.

[mauroservienti]

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.