fix(search): bound memory of the bleve descendant lookup - #3485
fix(search): bound memory of the bleve descendant lookup#3485n-at-han-k wants to merge 1 commit into
Conversation
searchResourcesByPath found a folder's descendants with a Path:<folder>/* wildcard. Path is a keyword field, one term per document, so bleve expanded the wildcard into one term searcher per descendant, all alive at once, each holding zapx dictionary structures and vellum FST readers. Peak live memory scaled with descendants x segments and the kernel OOM-killed the server on folder deletes (2.24 GB of a 2.29 GB live heap in this one stack on a production 7.5.0 instance). Enumerate the matching path terms from the field dictionary instead and fetch the documents in bounded batches of 500 exact term queries: same result set, O(500) live searchers, ~12x lower peak (1194 MB -> 102 MB at 100k docs), slightly faster. The RootID filter becomes an exact TermQuery so IDs with $/! no longer pass through the query parser; escapeQuery loses its last caller and is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UozPhD3wkKcnfisgoENx6G
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 46 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
This is a really good catch, I'm just not sure the approach is the best we can do. Let me cook something up - would be much appreciated, if you could give it a spin? |
Path is analyzed into its ancestor prefixes, like path_hierarchy in OpenSearch, so the descendant lookup behind delete/move/restore/purge, the scoped search and the KQL path predicate are one term query each instead of one term searcher per descendant (#1269, #3469). Descendants stream into the batch page by page. Schema 4 -> 5, v4 never shipped. Benchmark and memory-bound test adapted from #3485 by n-at-han-k.
|
Could you check #3495? |
|
I honestly couldn't comment. I'm not much of GO developer. |
|
@n-at-han-k that's honestly very valuable feedback. Thanks a lot. |
2 participants
fix(search): bound memory of the bleve descendant lookup
Description
Deleting, moving, restoring or purging a folder makes the search service
look up every descendant of that folder in the bleve index.
searchResourcesByPathdoes this with a wildcard query:Pathis a keyword field — one term per document, the full path — so bleveexpands this wildcard into one term searcher per descendant, all alive at
once, each holding zapx segment-dictionary structures and vellum FST readers.
Peak live memory scales with
descendants × segmentsand is superlinear inindex size. On a production instance a routine folder delete held 2.24 GB of
a 2.29 GB live heap in this one stack (
go tool pprof, opencloud 7.5.0):The process was OOM-killed by the kernel 112 times in one day on that host.
This is very likely the mechanism behind #1269 (comments describe OOM on
folder delete / trash emptying) and plausibly contributes to #3469.
The fix
Keep the same function, signature and result set, but enumerate the matching
path terms from the field dictionary (
FieldDictPrefix— an iterator, nosearchers) and fetch the documents in bounded batches of 500 exact
TermQuerys. Live searcher memory becomes O(500) instead of O(descendants).The
RootIDfilter moves fromQueryStringQueryto an exactTermQuery, soIDs containing
$/!no longer pass through the query parser.escapeQueryloses its last caller and is removed.
The opensearch backend already avoids this class of problem via its
path_hierarchyterm query; this brings the bleve backend in line.Benchmark
BenchmarkSearchResourcesByPath(added in this PR) builds an index of N filesin one folder and runs the descendant lookup. Cumulative allocations are
similar for any implementation that visits every descendant — what OOMs
servers is the peak live heap while the lookup runs, reported as
peak-MBvia a background
HeapInusesampler:Note the superlinear growth on main (5× docs → 6× peak) versus linear with the
fix — the gap keeps widening with index size.
Both variants return the identical result set (asserted inside the benchmark:
len(res) == N), and the existing package tests pass unchanged.Regression tests
descendants_test.go(added in this PR):TestSearchResourcesByPath— correctness edges the lookup must keep:1001 descendants (crosses the 500-term batch boundary twice), the folder
itself excluded, a sibling whose name shares the prefix (
./big2vs./big) excluded, the same path under another space's RootID excluded,and paths containing
* [ ] ? :and spaces.TestSearchResourcesByPathMemoryBounded— fails any implementation whoselive memory scales with folder size: 20k-doc folder, 64 MB peak-heap limit.
The former wildcard implementation holds ~168 MB here and fails; this
implementation peaks under ~20 MB. Skipped with
-short.End-to-end reproduction
Branch
memory-bug-testcontains a self-contained nix flake with two microVMs that run the full
scenario against real servers built from source — one unpatched, one carrying
only this patch, that being the only difference. Each VM uploads N files over
WebDAV, waits for search to index them, deletes the folder, and records the
opencloud cgroup at 1–10 Hz plus search-service pprof heap profiles to
repro-out/:Related Issue
closed as unreproducible; the benchmark above is the missing reproducer
How Has This Been Tested?
go test ./services/search/pkg/bleve/ ./services/search/pkg/search/— passthe wildcard implementation (168 MB > 64 MB limit) and passes on this one
without): 12k-file upload → index → folder delete over WebDAV; identical
end-to-end behaviour and search results, no regression
mainTypes of changes
Checklist:
🤖 Generated with Claude Code
https://claude.ai/code/session_01UozPhD3wkKcnfisgoENx6G