CenterForDigitalHumanities / CenterForDigitalHumanities/rerum_server_nodejs

`/search` paginates in application memory, so every page costs the server everything up to that page

Offen
#309 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
JavaScript
Sterne
3
Forks
6
Ø Merge
2 T. 47 Min.
Gemergte PRs (30 T.)
4

Beschreibung

## Summary

`/search` fans out to two Atlas Search indexes as two separate `db.aggregate()` calls, then merges, deduplicates, sorts, and slices the combined result in Node (`controllers/search.js:282`). Each branch caps at `$limit: limit + skip` (`controllers/search.js:211,216`), so a request for page 40 makes Atlas produce and return 4000 documents per index and makes Node process up to 8000 of them in order to hand back 100.

`/query` pushes the same work down to Mongo — `db.find(props).limit(limit).skip(skip)` (`controllers/crud.js:87`) — and its latency is flat with depth. `/search` cannot, and its latency is not.

Nothing about `$search` requires this. A single Atlas Search pipeline sorts by score natively, returns each document once, and can end in `$skip`/`$limit`. The blocker is the two-index fan-out, not the search stage.

This is the performance half of the `/search` work. The correctness half is #307, which should land first and independently — a client can page `/search` honestly once that lands, just not cheaply.

## Why this matters

**Cost scales with depth, not page size.** Dev, `{"searchText":"line"}`, `limit=100`:

```text
skip=0 -> 100 documents in 379ms
skip=1000 -> 100 documents in 664ms
skip=2000 -> 100 documents in 781ms
skip=3000 -> 100 documents in 1172ms
skip=3700 -> 100 documents in 2021ms
skip=3800 -> 25 documents in 1954ms
skip=3900 -> 0 documents in 1784ms
skip=5000 -> 0 documents in 1802ms
skip=20000 -> 0 documents in 1815ms
```

`/query` at the same depths, `{"__rerum.APIversion":{"$exists":true}}`, `limit=100`:

```text
skip=0 -> 100 documents in 340ms
skip=1000 -> 100 documents in 296ms
skip=5000 -> 100 documents in 323ms
skip=20000 -> 100 documents in 369ms
skip=99000 -> 100 documents in 445ms
```

The two seconds spent to return *nothing* at `skip=5000` is the cost of producing and merging the entire matching set. Cost plateaus once the result set is exhausted, which confirms the bound is `min(total matches, limit + skip)` per branch. Production shows the same shape at smaller scale: `line` has 574 matches there, and the empty pages at `skip=1000`, `2000`, and `20000` still cost roughly 850-900 ms each.

**The ceiling is the `skip` maximum.** At the effective maximum of 100000, a sufficiently broad term at maximum depth asks Node to hold on the order of 100000 full RERUM documents per branch in memory and sort them. Not identifiers — whole documents.

**It is a shared-process cost.** This work happens in the API process, not in Atlas. Under PM2 cluster mode a few concurrent deep searches contend for the same workers that serve every other endpoint.

## Affected lines

| File | Line | Current |
|------|------|---------|
| `controllers/search.js` | 82 | `buildDualIndexQueries()` builds two independent pipelines |
| `controllers/search.js` | 84, 153 | Two separate indexes: `presi3AnnotationText`, `presi2AnnotationText` |
| `controllers/search.js` | 211, 216 | `$limit: limit + skip` per branch — the depth cost |
| `controllers/search.js` | 32 | `mergeSearchResults()` deduplicates in Node, after the per-branch cap |
| `controllers/search.js` | 282, 368 | `merged.slice(skip, skip + limit)` — pagination in application memory |
| `controllers/search.js` | 446, 540, 672 | Same shape in the unmounted `searchFuzzily`, `searchWildly`, `searchAlikes` |
| `controllers/crud.js` | 87 | `/query` for contrast: `.limit(limit).skip(skip)` in Mongo |

## Proposed change

Three options, best first. This issue should settle which one before any code is written.

### 1. One combined Atlas Search index (recommended)

Define a single index covering both vocabularies — the IIIF 3.0 paths (`body.value`, `bodyValue`, and the `items` / `annotations` embedded documents) and the IIIF 2.1 paths (`resource.chars`, `resource.cnt:chars`, and the `resources` / `otherContent` / `sequences` embedded documents).

`/search` becomes one `$search` with all the existing `should` clauses, followed by `$skip` and `$limit`:

- **Ranking becomes native.** Atlas returns `$search` results in descending score order across all clauses, and every document is scored once against one index rather than compared across two.
- **Deduplication disappears.** Atlas returns each document exactly once; there is no key to derive and nothing to drop. `mergeSearchResults()` can be deleted, and with it both bugs from #307.
- **Paging runs in the database**, so cost stops scaling with depth.

Cost: an Atlas-side index change, and both index generations must exist during the transition.

### 2. `$unionWith` in a single pipeline

Keep both indexes, run the IIIF 2.1 branch as a `$unionWith` sub-pipeline against the same collection, then `$group` on `_id`, `$sort` by score, and `$skip`/`$limit` — all server-side. Grouping on `_id` in Mongo compares structurally, so embedded-object ids group correctly there.

This moves the work off the API process but not off the cluster: the `$group` is a blocking stage over the union, so Atlas still materializes the merged set. Smaller change, smaller payoff. **Confirm `$search` is permitted as the first stage of a `$unionWith` sub-pipeline on our cluster tier before committing to this** — it is not universally available.

### 3. Keep the shape, bound the damage

If neither restructuring is scheduled soon, enforce a `/search`-specific `skip` maximum far below the `/query` maximum, since the two endpoints pay very different prices for the same depth. This is a mitigation, and it makes the two endpoints inconsistent — exactly what the shared-contract work argues against — so it should be a deliberate, documented decision rather than a default.

## Notes

- **Breaking.** Option 1 changes relevance scoring, and should: a single index scores a document once across all matched clauses. Today the two branches' scores are never compared against each other at all, so ordering will shift substantially. That is the point, not a regression. Membership should change only by the currently dropped documents reappearing.
- Depends on #308. Reviewing a new index definition against two that exist only in a browser tab is not a review.
- Should land after #307, which is the cheap correctness fix and is not blocked by anything. If option 1 ships, it supersedes both of that issue's fixes — but it should not gate them.
- `rel="next"` (#302) needs `/search` to over-fetch one record. Under the current shape that is `limit + skip + 1` per branch; under option 1 it becomes the ordinary single-pipeline `limit + 1`.
- Once this lands, `/search` can take a real cursor. Atlas Search's `searchAfter` / `searchSequenceToken` is the keyset equivalent for `$search` results — **confirm availability on our cluster tier and server version before planning it.** See #303 for why a cursor before this lands would be dishonest.
- Whatever lands must keep the property already verified: sequential pages equal to a single larger request, in the same order.

## Acceptance criteria

- [ ] An option is chosen and the reasoning is recorded on the issue before implementation
- [ ] `/search` pagination is applied in MongoDB rather than in application memory
- [ ] `/search` latency at depth is flat with respect to `skip`, comparable to `/query`, measured with the same probes used above
- [ ] Every document matched by the search is reachable by paging, including documents whose `_id` is an embedded object
- [ ] Results are ordered by descending relevance score, verified by asserting the score sequence never increases
- [ ] Sequential paged results still equal a single larger request, in the same order
- [ ] Relevance ordering change is measured on representative queries and accepted before cutover
- [ ] `/search` and `/search/phrase` both covered, and the unmounted search variants updated to match
- [ ] The ordering guarantee is stated in `openapi/contracts/core-provider.openapi.yaml`, not only in JSDoc

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne mit controllers/search.js, insbesondere mit buildDualIndexQueries(), mergeSearchResults() und den Handlern /search und /search/phrase; vergleiche deren Pagination mit controllers/crud.js:87. Untersuche die Indexdefinitionen aus #308 und den Vertrag in openapi/contracts/core-provider.openapi.yaml und führe anschließend die dokumentierten Deep-Page-Probes aus. Erledigt ist die Aufgabe, wenn eine abgestimmte Option implementiert und gemessen wurde und die Abdeckung für Sortierung, IDs eingebetteter Objekte, aufeinanderfolgende Seiten, beide gemounteten Endpunkte und die nicht gemounteten Varianten vorhanden ist.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
javascript, mongodb
Bereich
api, backend, database, performance
Issue-Typ
Refactoring
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Aktiv
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
35/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.