Add a self-contained v8 search schema revision (PP-4659)

[jonathangreen]

Description

Adds a self-contained v8 OpenSearch schema revision (SearchV8) and registers it in the revision directory.

Unlike earlier revisions, which chained off one another (SearchV7 -> SearchV6 -> SearchV5), v8 subclasses SearchSchemaRevision directly and defines its complete mapping (analyzers, filters, and fields) inline. Breaking the chain means each revision stands alone, so an old revision's module can simply be deleted once nothing in production is using it.

Schema-wise v8 is equivalent to v7 (the full v5 mapping plus v6's lane_priority_level and v7's licensepools.last_updated). The mapping is unchanged; the differences are all in the index settings, which v8 now pins explicitly so a newly created index is fully deterministic rather than relying on inherited cluster defaults:

• number_of_shards = 1 — earlier indexes inherited a 5-primary-shard count from the original Elasticsearch 6.x defaults, carried forward through every reindex because nothing set it explicitly. Per-library indexes are well under a gigabyte, far below the per-shard target for search workloads, so a single primary shard is correct. This setting is immutable after index creation, so it must be set up front.
• number_of_replicas = 1 — matches what every production index already runs and is also the OpenSearch default, so this is operationally a no-op. It is a dynamic setting and can still be retuned at runtime for a larger topology; it is pinned only so indexes are created with a known replica count.
• index.search.slowlog.threshold.* — seeds slow-query-log thresholds on every index this revision creates, so slow query- and fetch-phases are written to the cluster slow log. Paired with the SEARCH_SLOW_LOGS publishing wired up in the hosting playbook, these entries reach the domain's CloudWatch log group. These are dynamic settings, so they establish a baseline at index creation that can still be retuned on a live index without a reindex.

Motivation and Context

The revision chain meant no old schema version could be removed without breaking the revisions built on top of it, so deprecated versions accumulated indefinitely. Making each revision self-contained lets old versions be deleted once production no longer uses them.

Pinning the index settings replaces inherited, undocumented cluster defaults with an explicit, deterministic configuration — most importantly the shard count, which is immutable after index creation and had been silently carried over from the Elasticsearch era.

JIRA: PP-4659

How Has This Been Tested?

Added tests/manager/search/test_search_v8.py, covering the version number, registration in the revision directory, the pinned shard/replica counts, and the seeded slow-query-log thresholds in the generated mapping document. Ran the search test suite under the docker tox environment.

Checklist

• I have updated the documentation accordingly.
• All new and existing tests passed.

[claude]

Claude finished @jonathangreen's task in 2m 12s —— View job

---

Code Review

• Read changed files (v8.py, revision_directory.py, test_search_v8.py)
• Compared against v5/v6/v7, the base revision, and SearchMappingDocument
• Traced how index settings flow into index_create
• Posted review

Summary

This is a clean, well-scoped change. The self-contained design is sound, the mapping faithfully reproduces v7 (the test_mapping_matches_v7 round-trip is a great guard against accidental drift), and the new index settings are serialized into the create body correctly — the dotted slowlog keys nested under index flatten to valid index.search.slowlog.threshold.* settings, and putting the immutable number_of_shards in the create body is the right place for it. I agree with your calls on the two Greptile comments: re-flagging the posessive typo and the mutable class constants would introduce a divergence from v5/v6/v7, which is worse than preserving them verbatim in a deliberately faithful copy. I have one minor suggestion on the new test.

Details

Minor: tests/manager/search/test_search_v8.py:18-26

test_pins_index_settings only asserts that each setting key is present, not what it's set to. The central, hard-to-reverse claim of this PR is number_of_shards == 1 (immutable after index creation), yet this test would still pass if someone later set it to 5. Asserting the values would lock in the thing the PR exists to guarantee:

assert index["number_of_shards"] == 1
assert index["number_of_replicas"] == 1
assert index["search.slowlog.threshold.query.warn"] == "3s"

circulation/tests/manager/search/test_search_v8.py

Lines 18 to 26 in 77650c4

	for setting in [
	"number_of_shards",
	"number_of_replicas",
	"search.slowlog.threshold.query.warn",
	"search.slowlog.threshold.query.info",
	"search.slowlog.threshold.fetch.warn",
	"search.slowlog.threshold.fetch.info",
	]:
	assert setting in index

[greptile-apps]

Greptile Summary

This PR introduces SearchV8, a self-contained OpenSearch schema revision that directly extends SearchSchemaRevision instead of chaining off SearchV7, and registers it in SearchRevisionDirectory. The mapping itself is functionally identical to v7; the novelty is pinning index settings (number_of_shards, number_of_replicas, slowlog thresholds) explicitly so newly created indexes are fully deterministic.

• New revision class (v8.py): breaks the inheritance chain (v5 → v6 → v7) by subclassing SearchSchemaRevision directly and inlining the complete mapping, allowing future deletion of old revisions without cascading breakage.
• Explicit index settings: pins the immutable shard count to 1 (matching real-world library index sizes) and seeds slow-query-log thresholds at index creation time so they flow into CloudWatch via the hosting playbook's SEARCH_SLOW_LOGS wiring.
• Tests (test_search_v8.py): verify the version number, direct-inheritance invariant, presence of all pinned settings keys, and that the new revision's field mappings and analysis configuration are byte-for-byte identical to v7's.

Confidence Score: 5/5

Safe to merge. The new revision is self-contained, the mapping is byte-for-byte identical to v7 (verified by the fidelity test), and the only behavioural difference — explicit index settings — is intentional and tested.

The implementation is a faithful, self-contained copy of the v7 mapping with the addition of pinned index settings. The logic is straightforward, tests are comprehensive (version number, inheritance invariant, settings key presence, and full field/analysis parity with v7), and there are no data-path changes that could affect production search behaviour. Previously flagged issues (mutable class constants, the posessive typo) were deliberately retained for mapping fidelity with the older schema chain.

No files require special attention.

Important Files Changed

Filename	Overview
src/palace/manager/search/v8.py	Introduces the self-contained SearchV8 revision with pinned index settings (shard count, replicas, slowlog thresholds) and an inlined copy of the v5/v6/v7 field mapping and analysis configuration. No logic defects found; mutable class-level constants and the posessive typo were acknowledged in previous review threads and intentionally preserved for mapping fidelity.
src/palace/manager/search/revision_directory.py	Adds the SearchV8 import and appends a SearchV8() instance to the REVISIONS list. Change is minimal and correct; the directory's uniqueness check will catch any future version collision.
tests/manager/search/test_search_v8.py	Covers version number, direct-inheritance invariant, presence of all pinned settings keys, and byte-for-byte field/analysis parity with v7. The import of SearchV7 in test_mapping_matches_v7 creates a link that must be removed when v7 is eventually deleted, but this is an expected consequence of the fidelity test rather than a defect.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class SearchSchemaRevision {
        <<abstract>>
        +version() int
        +mapping_document() SearchMappingDocument
        +name_for_index(base_name) str
    }

    class SearchV5 {
        +version() int
        +mapping_document() SearchMappingDocument
    }

    class SearchV6 {
        +version() int
    }

    class SearchV7 {
        +version() int
    }

    class SearchV8 {
        +version() int
        +NUMBER_OF_SHARDS: int
        +NUMBER_OF_REPLICAS: int
        +SEARCH_SLOWLOG_THRESHOLDS: dict
        +CHAR_FILTERS: dict
        +AUTHOR_CHAR_FILTER_NAMES: list
        +mapping_document() SearchMappingDocument
    }

    SearchSchemaRevision <|-- SearchV5 : OLD CHAIN
    SearchV5 <|-- SearchV6 : OLD CHAIN
    SearchV6 <|-- SearchV7 : OLD CHAIN
    SearchSchemaRevision <|-- SearchV8 : SELF-CONTAINED

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
classDiagram
    class SearchSchemaRevision {
        <<abstract>>
        +version() int
        +mapping_document() SearchMappingDocument
        +name_for_index(base_name) str
    }

    class SearchV5 {
        +version() int
        +mapping_document() SearchMappingDocument
    }

    class SearchV6 {
        +version() int
    }

    class SearchV7 {
        +version() int
    }

    class SearchV8 {
        +version() int
        +NUMBER_OF_SHARDS: int
        +NUMBER_OF_REPLICAS: int
        +SEARCH_SLOWLOG_THRESHOLDS: dict
        +CHAR_FILTERS: dict
        +AUTHOR_CHAR_FILTER_NAMES: list
        +mapping_document() SearchMappingDocument
    }

    SearchSchemaRevision <|-- SearchV5 : OLD CHAIN
    SearchV5 <|-- SearchV6 : OLD CHAIN
    SearchV6 <|-- SearchV7 : OLD CHAIN
    SearchSchemaRevision <|-- SearchV8 : SELF-CONTAINED

Loading

_{Reviews (2): Last reviewed commit: "Trim comments and tests" | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.46%. Comparing base (c1332fe) to head (77650c4).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3522      +/-   ##
==========================================
+ Coverage   93.44%   93.46%   +0.01%     
==========================================
  Files         511      512       +1     
  Lines       46483    46561      +78     
  Branches     6343     6344       +1     
==========================================
+ Hits        43437    43516      +79     
+ Misses       1969     1968       -1     
  Partials     1077     1077              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tdilauro]

Looks great! 🥇

[jonathangreen]

I'm going to leave this one unmerged until I get back from vacation, so I'm around when it rolls out, in case the new shard settings cause any issues.