fix(search): hierarchy tokenizer for bleve path fields - #3510
Merged
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 21 |
🟢 Coverage 77.67% diff coverage · +0.05% coverage variation
Metric Results Coverage variation ✅ +0.05% coverage variation (-1.00%) Diff coverage ✅ 77.67% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (e503c2c) 88539 20997 23.71% Head commit (9cc7757) 88613 (+74) 21062 (+65) 23.77% (+0.05%) 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 (#3510) 103 80 77.67% 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
dschmidt
marked this pull request as ready for review
September 10, 2026 17:34
1 task
fschade
reviewed
Sep 10, 2026
dschmidt
force-pushed
the
fix/bleve-path-performance
branch
2 times, most recently
from
September 10, 2026 18:42
d20d3a0 to
66f992d
Compare
Path was a keyword, so the descendant lookup behind delete/move/restore/purge, the scoped search and the KQL path predicate expanded into one term searcher per descendant and OOM-killed the server on large folders (#1269, #3469). Path is now analyzed into its ancestor prefixes, like path_hierarchy in OpenSearch: ./a/b.txt becomes ., ./a, ./a/b.txt. A folder's descendants are every document carrying the folder's path as a term, so all three call sites are a single term query. Schema 4 -> 5, v4 never shipped. The same tokenizer with tag_depth is registered as the geohash analyzer, so #3272 can add its geohash field without another schema change.
dschmidt
force-pushed
the
fix/bleve-path-performance
branch
from
September 10, 2026 18:42
66f992d to
9cc7757
Compare
fschade
approved these changes
Sep 11, 2026
fschade
enabled auto-merge
September 11, 2026 07:48
github-project-automation
Bot
moved this from In Progress
to Done
in OpenCloud Team Board
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
3 participants
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.
Description
Path in bleve was a keyword, so the descendant lookup (delete/move/restore/purge), the scoped search and the KQL
path:predicate expanded into one term searcher per descendant and OOM-killed the server on large folders.Path is now analyzed into its ancestor prefixes (
./a/b.txt→.,./a,./a/b.txt), likepath_hierarchyin OpenSearch. All three call sites are a single term query. Schema 4 → 5 (v4 never shipped).The same tokenizer with
tag_depthis registered as thegeohashanalyzer so #3272 can add its field additively. Minimal version of #3495: no descendant streaming, no geohash sibling.Peak live heap, descendant lookup at 100k files: 1217 MB → 133 MB.
Related Issue
How Has This Been Tested
Types of changes
This is an (imho) important performance fix. We've broken the index scheme for the next release already so now is the PERFECT time to get it in.
I've separated the breaking parts from my original PR into this. This already gets us a 10x less memory usage effect. So I would strongly suggest to merge it for the 8.0.0 release. The other improvements are non breaking and can happen at another time in the 8.x cycle.
BTW: I don't have any other breaking changes planned (alas this wasn't planned but it was brought up and debugged just in time by another user/contributor).
I put everything breaking into my refactor PR, which I separated out of a feature branch with all my stuff merged together and working. So I'm pretty confident that everything I've been working on can be added gradually during the 8.x series without a single breaking change.