feat(search): hierarchical path and geohash terms for bleve - #3495
Open
dschmidt wants to merge 3 commits into
Open
feat(search): hierarchical path and geohash terms for bleve#3495dschmidt wants to merge 3 commits into
dschmidt wants to merge 3 commits into
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 83 |
🟢 Coverage 85.71% diff coverage · +0.20% coverage variation
Metric Results Coverage variation ✅ +0.20% coverage variation (-1.00%) Diff coverage ✅ 85.71% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (b18618a) 88539 20985 23.70% Head commit (fc7658b) 88607 (+68) 21179 (+194) 23.90% (+0.20%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3495) 203 174 85.71% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
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.
dschmidt
changed the title
There was a problem hiding this comment.
🟢 Approval recommended
The schema, query, batching, and regression-test changes consistently implement bounded hierarchical path lookup.
Pull request overview
Adds hierarchical Bleve path indexing to prevent descendant queries from consuming memory proportional to folder size.
Changes:
- Adds and tests a reusable hierarchy tokenizer.
- Replaces wildcard path expansion with single-term queries.
- Streams batch operations through bounded, ID-paginated searches and bumps the schema version.
File summaries
| File | Description |
|---|---|
services/search/pkg/search/search.go |
Bumps the shared search schema to v5. |
services/search/pkg/query/bleve/compiler.go |
Compiles path predicates as term queries. |
services/search/pkg/query/bleve/compiler_test.go |
Updates path-query expectations. |
services/search/pkg/mapping/opts.go |
Defines the Bleve path analyzer name. |
services/search/pkg/mapping/bleve.go |
Assigns the hierarchy analyzer to path fields. |
services/search/pkg/bleve/testdata/mapping.golden.json |
Records the updated mapping. |
services/search/pkg/bleve/index.go |
Registers hierarchy analysis and paginates descendant lookup. |
services/search/pkg/bleve/hierarchy/hierarchy.go |
Implements the hierarchy tokenizer. |
services/search/pkg/bleve/hierarchy/hierarchy_test.go |
Tests tokenization, querying, and facets. |
services/search/pkg/bleve/hierarchy/hierarchy_suite_test.go |
Adds the tokenizer test suite. |
services/search/pkg/bleve/descendants_test.go |
Tests correctness, pagination, and bounded memory. |
services/search/pkg/bleve/descendants_bench_test.go |
Benchmarks descendant lookup memory. |
services/search/pkg/bleve/bleve.go |
Removes obsolete query escaping. |
services/search/pkg/bleve/batch.go |
Streams folder operations over descendants. |
services/search/pkg/bleve/backend.go |
Uses hierarchy terms for scoped searches. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dschmidt
changed the title
dschmidt
force-pushed
the
feat/search-hierarchy-tokenizer
branch
from
September 8, 2026 16:07
61c3aa1 to
f9f9a8a
Compare
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.
The same tokenizer with tag_depth turns a geohash into depth-tagged prefixes, so a terms facet with TermPrefix is a geohash grid at that precision. One location_geohash field per geopoint, part of the v5 schema so #3272 needs no further bump. OpenSearch maps it unindexed, geohash_grid works on the geo_point.
dschmidt
marked this pull request as ready for review
September 8, 2026 16:09
5 tasks
…tors on Path, path value normalized once - the walker yields the folder itself, the batch skips it by id so a root re-indexed under its old path is never treated as its own descendant - descendant page 20k: every page re-sorts the full match set, 20k trades ~19 MB live heap for 4x fewer rescans - the memory spec samples /gc/heap/live:bytes instead of HeapInuse, which measured GC pacing and failed with GOGC=off; page crossing is tested for move, delete, restore and purge - Path drops term vectors, nothing highlights or phrase-queries it - KQL path values are trimmed and an empty value means the root in the normalizer, for both engines - the tokenizer skips empty segments (leading or doubled delimiter)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Path in bleve was a keyword, so descendant lookups (delete/move/restore/purge), the scoped search and the KQL
path:predicate expanded into one term searcher per descendant. On large folders that held gigabytes of live heap and OOM-killed the server (#1269, #3469, diagnosis in #3485).Path. A new
hierarchytokenizer indexes every ancestor of a path as its own term:./a/b.txtbecomes.,./a,./a/b.txt. This is what OpenSearch's built-inpath_hierarchyanalyzer does, and OpenSearch already used it for Path. A folder's descendants are now every document carrying the folder's path as a term, so all three call sites become a single term query. The batch operations additionally stream the descendants in pages of 20k instead of collecting them.Geohash. #3272 adds a geohash grid aggregation. OpenSearch has it natively on the geo_point. bleve does not, and #3272 emulates it with twelve keyword fields, one per precision. The same tokenizer replaces those: with
tag_depthit turns a geohash into1/u,2/u4,3/u4p, ... and a terms facet restricted toTermPrefix: "5/"yields exactly the precision-5 cells. Since this PR rebuilds the index anyway, thegeohashanalyzer and alocation_geohashfield next tolocation_geopointare added here, so #3272 can be rebuilt on them without breaking the schema again. The field is a single 12-character geohash per geopoint. In OpenSearch it is mapped but not indexed, only bleve needs it.Schema 4 -> 5. v4 never shipped.
Measured at 100k descendants (
BenchmarkSearchResourcesByPathand a scoped search returning 10 hits):Benchmark and memory-bound test adapted from #3485 by @n-at-han-k.