From e78a773c8af26bf0c36a181c6eee2bd19a5611d6 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Mon, 24 Aug 2026 18:38:10 -0300 Subject: [PATCH 1/7] docs(content-drive): spec for field-filter chunk multiplier (#37184) --- .../spec.md | 199 ++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 specs/37184-content-drive-field-filter-chunk-multiplier/spec.md diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md new file mode 100644 index 000000000000..5bd1db06363a --- /dev/null +++ b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md @@ -0,0 +1,199 @@ +# Feature Specification: Content Drive Field-Filter Chunk Multiplier + +**Feature Branch**: `37148-field-filter-chunk-multiplier` + +**Created**: 2026-08-24 + +**Status**: Draft + +**Type**: Performance Fix + +**Related GitHub Issue**: [#37184](https://github.com/dotCMS/core/issues/37184). Parent epic: [#36814](https://github.com/dotCMS/core/issues/36814). Originally investigated as item 2 of [#37148](https://github.com/dotCMS/core/issues/37148); item 1 "candidate-scan query" ([#37183](https://github.com/dotCMS/core/issues/37183)) and item 4 "per-row user lookup" ([#37186](https://github.com/dotCMS/core/issues/37186)) are tracked and spec'd as separate sibling issues. + +**Input**: User description: "Content Drive — Item 2: eliminate the field-filter chunk multiplier in BrowserAPIImpl. Field filters that route entirely to the search index still re-run the expensive database candidate-scan query multiple times per request instead of once, because the DB-first hybrid chunk loop keeps iterating even when there is no database-routed criterion left to protect." + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Filtering a large folder by a content-type field returns promptly (Priority: P1) + +A content author or administrator browsing a folder with thousands of items applies a filter on +a content-type field (for example, a Text, Select, Date-range, Multi-select, or Category field). +Today, once the filter is entirely resolvable against the search index, the system still repeats +its full database scan of the folder's contents multiple times before it has gathered enough +matches to fill a page — on a ~20,000-item folder this took roughly four repeats and made the +response six times slower than an equivalent search. The user should instead get their filtered +results back in roughly the time a comparable search takes, without repeated re-scanning. + +**Why this priority**: This is the dominant remaining performance gap Content Drive has against +its baselines once the highest-cost item (item 1) is addressed; field filtering over large +folders is a normal, frequent authoring action. + +**Independent Test**: Can be fully tested by applying a single content-type field filter (no Tag, +no Relationship, no workflow, no free-text term) against a folder with a large number of children +and confirming both that the number of times the underlying candidate scan runs drops to at most +one, and that the returned result set is unchanged from today's behavior. + +**Acceptance Scenarios**: + +1. **Given** a folder with ~20,000 children and a filter on a Text field that matches 40 items, + **When** the user applies the filter, **Then** the system scans the folder's candidate content + at most once (not four times as today) and returns the same 40 matching items. +2. **Given** the same folder and filter, **When** the user applies a Date-range, Multi-select, or + Category filter instead, **Then** results are identical to today's behavior and the same + single-scan improvement applies. +3. **Given** a small folder (a few dozen children) with the same kind of field filter, **When** + the user applies it, **Then** results are unchanged and response time does not regress. + +--- + +### User Story 2 - Filters that must stay database-first keep working exactly as before (Priority: P2) + +A user filters by a Tag field, a Relationship field, a workflow scheme/step, or a free-text +search term — alone or combined with a content-type field filter. These criteria must keep being +resolved against the database so that content the user (or a teammate) just saved is immediately +visible and correctly filtered, even if the search index has not caught up yet. This behavior must +not change as a side effect of speeding up the pure-field-filter case. + +**Why this priority**: This protects the read-your-writes guarantee that Content Drive depends on +for Tag, Relationship, workflow, and text filtering; regressing it would reintroduce the exact +"I saved it and it vanished" complaint the current database-first design exists to prevent. + +**Independent Test**: Can be fully tested by combining a content-type field filter with a Tag +filter (and separately with a Relationship filter, and separately with a workflow filter, and +separately with a free-text term) and confirming the result set and the number of database scans +are unchanged from today's behavior in every combination. + +**Acceptance Scenarios**: + +1. **Given** a folder and a search that combines a content-type field filter with a Tag filter, + **When** the user applies it, **Then** the result set is unchanged from today and the request + still resolves through the existing database-first path. +2. **Given** the same setup but with a Relationship filter instead of Tag, **When** the user + applies it, **Then** behavior is unchanged. +3. **Given** a search that adds a workflow scheme/step filter on top of a content-type field + filter, **When** the user applies it, **Then** behavior is unchanged. +4. **Given** a search that adds a free-text search term on top of a content-type field filter, + **When** the user applies it, **Then** behavior is unchanged (the free-text path continues to + combine database and index results as it does today). + +### Edge Cases + +- What happens when the field filter matches zero items in a large folder? The system should + still resolve this in a single pass rather than exhausting every chunk before concluding there + are no matches. +- What happens when a field filter is combined with pagination (deep pages)? Paging behavior and + page-to-page consistency must be unchanged from today. +- What happens when the folder itself is very large and permission filtering removes most + candidates? The mandatory read-permission check still runs after candidates are retrieved, on + every path, so a permission-restricted user never sees content they cannot read. +- What happens on an empty folder, or a folder with no items matching the filter at all? The + system returns an empty result without unnecessary repeated scanning. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: System MUST determine, for each field-filter search request, whether every + requested field criterion is resolvable purely against the search index (i.e., no criterion + requires the database to preserve immediate visibility of recent writes). +- **FR-002**: When that condition holds, and the request has no workflow filter and no free-text + or file-name term, System MUST resolve the request without repeatedly re-scanning the database + candidate set — the underlying database scan MUST execute at most once per request, regardless + of folder size or how sparse the matches are. +- **FR-003**: The single-pass resolution in FR-002 MUST still be scoped to the folder (and site) + the user is browsing — it must never return content from outside the requested folder. +- **FR-004**: System MUST still apply the mandatory read-permission filter, sourced from the + database, after candidates are retrieved, for every request — this fix MUST NOT weaken or skip + permission filtering to gain speed. +- **FR-005**: When any criterion requires database resolution to preserve immediate visibility of + recent writes (a Tag filter or a Relationship filter), OR a workflow filter is present, OR a + free-text/file-name term is present, System MUST behave exactly as it does today: full + database-first resolution, unchanged result sets, unchanged number of scans. +- **FR-006**: The database-first resolution strategy MUST remain the default behavior for every + request shape not covered by FR-002; this fix MUST NOT change what the default search strategy + is for the general case. +- **FR-007**: Result sets returned for field filters on Text, Date-range, Multi-select/Checkbox, + Tag, and Category fields MUST be unchanged from current behavior (same items, same order, same + pagination behavior). +- **FR-008**: Combining a content-type field filter with a Tag filter MUST continue to return the + same correct results as today. +- **FR-009 — RESOLVED (2026-08-24): no dedicated kill switch.** No independently-toggleable + operator flag for the single-pass behavior in FR-002. Decision reasoning: the freshness + trade-off it would guard (see Assumptions) is narrow and scoped — it applies only when zero + database-required criteria are present — and mirrors a trade-off ADR-0018 already accepts by + default for free-text search, without a dedicated flag for that case either. The existing + general search-strategy configuration (`BROWSE_API_HEURISTIC_TYPE`) remains the escape hatch of + last resort if the single-pass path needs to be disabled entirely. + +### Key Entities + +- **Field-filter search request**: A Content Drive request that narrows results by one or more + content-type field values (e.g., a Text field "contains", a Date range, a Multi-select "in + list", a Tag, or a Category), scoped to a folder/site, language, and the requesting user's + permissions. +- **Field criterion**: A single field-level condition within a request, distinguished by whether + it must be resolved against the database (Tag, Relationship) to preserve immediate visibility of + recent writes, or may be resolved against the search index (Text, Select, Boolean, Date, + Category). + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: For a field filter with no database-required criteria over a folder with roughly + 20,000 items, the number of times the system re-scans the folder's candidate content drops from + four to at most one. +- **SC-002**: Response time for that same case falls to within 20% of the response time of the + closest equivalent content-search operation (matching the threshold already used to evaluate + Content Drive elsewhere in this investigation). +- **SC-003**: 100% of tested combinations of a field filter with a Tag filter, a Relationship + filter, a workflow filter, or a free-text term return results identical to today's behavior. +- **SC-004**: 100% of tested field-filter-only cases (Text, Date-range, Multi-select, Category) + return the same result sets as today, across small, medium, and large folders. + +## Legacy Considerations *(dotCMS-specific — mandatory)* + +- **Existing behavior touched**: The Content Drive / Site Browser search backend + (`com.dotcms.browser` package — modern, not legacy code) that decides whether a search is + resolved against the database, the search index, or a hybrid of both. +- **Backward-compatibility expectations**: The API contract, response shape, and result + correctness for every existing filter combination must be unchanged. Only the internal number of + database round-trips for the specific case described in FR-002 changes. No database schema, no + index mapping, and no REST contract changes are anticipated. +- **Known related decisions**: ADR-0018 (database-first search for Content Drive, with text + filtering deferred to the search index) is binding on this work. It requires that structural and + metadata filtering stay database-first always, that the database-first hybrid strategy remain + the default, and that the index-only strategy never become the default. This fix operates + strictly inside that contract: it changes how many times the existing hybrid strategy's database + step runs for one already-correctly-routed case, not which source of truth is authoritative for + any criterion. Whether structural/metadata filtering should ever move to the index is already + settled by ADR-0018 and is out of scope for this spec. + +## Dependencies & Coordination + +- **Shared query with item 1 (separate spec, out of scope here)**: A separate, parallel effort + (issue #37148 item 1) is reworking the same database candidate-scan query this fix's chunk loop + currently calls repeatedly, to fix an unrelated planner-instability problem on very large + folders. This fix reduces how often that query is invoked for the field-filter-only case + addressed here, which lowers this case's exposure to item 1's problem, but does not fix it. Item + 1's eventual change to that query must continue to behave correctly when invoked at most once + per request, as this fix will do for its case. Coordinate before either change merges independently. + +## Assumptions + +- A field filter is considered fully index-resolvable when none of its criteria are a Tag or a + Relationship field (the two field types the codebase already resolves against the database to + preserve immediate visibility of recent writes); this mirrors the existing, documented routing + rule and is not a new judgment call introduced by this fix. +- **RESOLVED (2026-08-24), no product sign-off required.** Accepting a brief, index-lag-bounded + delay before a just-written item appears in a field-filter-only search (no Tag, no + Relationship, no workflow, no free-text term) is a deliberate, scoped trade-off for this one + case, mirroring the trade-off ADR-0018 already accepts by default for free-text search — it is + not a general relaxation of Content Drive's read-your-writes guarantee, which continues to hold + for every other filter combination. Treated as a technical decision, not a product one, because + it extends an already-accepted architectural trade-off to one additional, narrowly-scoped case + rather than introducing a new one. Ships as default behavior (see FR-009 — no dedicated kill + switch either). +- The existing per-field index query logic (already used by the current hybrid strategy) is + assumed to be reusable as-is for the single-pass case; no new field-to-index translation logic is + expected to be needed. From b9cce8c5428aa997e55b89e3b7433ed83fe551dd Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Mon, 31 Aug 2026 21:42:06 -0300 Subject: [PATCH 2/7] docs(content-drive): address review feedback on field-filter chunk multiplier spec --- .../spec.md | 68 +++++++++++++------ 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md index 5bd1db06363a..c4841ebfba19 100644 --- a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md +++ b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md @@ -1,6 +1,6 @@ # Feature Specification: Content Drive Field-Filter Chunk Multiplier -**Feature Branch**: `37148-field-filter-chunk-multiplier` +**Feature Branch**: `issue-37184-content-drive-field-filter-chunk-multiplier` **Created**: 2026-08-24 @@ -96,10 +96,19 @@ are unchanged from today's behavior in every combination. - **FR-001**: System MUST determine, for each field-filter search request, whether every requested field criterion is resolvable purely against the search index (i.e., no criterion requires the database to preserve immediate visibility of recent writes). -- **FR-002**: When that condition holds, and the request has no workflow filter and no free-text - or file-name term, System MUST resolve the request without repeatedly re-scanning the database - candidate set — the underlying database scan MUST execute at most once per request, regardless - of folder size or how sparse the matches are. +- **FR-002 — implementation named (2026-08-31, per review): hybrid single-scan, not + index-only.** When that condition holds, and the request has no workflow filter and no + free-text or file-name term, System MUST resolve the request without repeatedly re-scanning + the database candidate set — the underlying database scan MUST execute at most once per + request, regardless of folder size or how sparse the matches are. This is a **hybrid** fix: + the existing database-first candidate scan still runs (fixing only its repeat-execution + count from ~4 down to 1), it does not switch to `buildPureESQuery`/index-only resolution. + That alternative was considered and rejected: `buildPureESQuery` (`BrowserAPIImpl.java:605`) + has no folder/parentPath filter today (only host-level scoping), and moving structural + filtering there would conflict with this spec's own cited ADR-0018 requirement that + structural/metadata filtering stay database-first always (see Legacy Considerations). FR-003, + FR-004, and FR-007 below describe *preserving* existing database-sourced behavior, not + building new index-side folder/permission/pagination logic. - **FR-003**: The single-pass resolution in FR-002 MUST still be scoped to the folder (and site) the user is browsing — it must never return content from outside the requested folder. - **FR-004**: System MUST still apply the mandatory read-permission filter, sourced from the @@ -117,13 +126,18 @@ are unchanged from today's behavior in every combination. pagination behavior). - **FR-008**: Combining a content-type field filter with a Tag filter MUST continue to return the same correct results as today. -- **FR-009 — RESOLVED (2026-08-24): no dedicated kill switch.** No independently-toggleable - operator flag for the single-pass behavior in FR-002. Decision reasoning: the freshness - trade-off it would guard (see Assumptions) is narrow and scoped — it applies only when zero - database-required criteria are present — and mirrors a trade-off ADR-0018 already accepts by - default for free-text search, without a dedicated flag for that case either. The existing - general search-strategy configuration (`BROWSE_API_HEURISTIC_TYPE`) remains the escape hatch of - last resort if the single-pass path needs to be disabled entirely. +- **FR-009 — RESOLVED (2026-08-24, correction 2026-08-31): no dedicated kill switch, and no + existing config already covers this.** No independently-toggleable operator flag for the + single-pass behavior in FR-002. Decision reasoning: the freshness trade-off it would guard + (see Assumptions) is narrow and scoped — it applies only when zero database-required criteria + are present — and mirrors a trade-off ADR-0018 already accepts by default for free-text + search, without a dedicated flag for that case either. Correction: `BROWSE_API_HEURISTIC_TYPE` + (`SearchHeuristicType`, `BrowserAPIImpl.java:465-470`) does **not** apply here — it toggles + between `HYBRID_SINGLE_CHUNKED_QUERY_ES` and `PURE_ES` for the free-text-filtering code path + (`doElasticSearchTextFiltering`) only, and has no effect on `getContentByChunks`'s repeat-scan + loop, which is what FR-002 changes. There is genuinely no existing config-level escape hatch + for this fix; if the single-pass path ever needs disabling, that would require a new flag + added at that time, not one that exists today. ### Key Entities @@ -145,7 +159,13 @@ are unchanged from today's behavior in every combination. four to at most one. - **SC-002**: Response time for that same case falls to within 20% of the response time of the closest equivalent content-search operation (matching the threshold already used to evaluate - Content Drive elsewhere in this investigation). + Content Drive elsewhere in this investigation). **Dependency**: because FR-002 is a + hybrid single-scan fix (the database candidate-scan query still runs once, per FR-002's + resolution above), this target is only reachable once that single scan itself is fast on + large folders — i.e., once issue #37148 item 1's fix (spec'd separately in #37230, materialized + folder-first CTE) has landed. Before #37230 lands, a single scan over a ~20,000-item folder is + still expected to take roughly the same ~470-490ms that motivated #37230, so SC-002 cannot be + validated in isolation from that dependency. - **SC-003**: 100% of tested combinations of a field filter with a Tag filter, a Relationship filter, a workflow filter, or a free-text term return results identical to today's behavior. - **SC-004**: 100% of tested field-filter-only cases (Text, Date-range, Multi-select, Category) @@ -171,13 +191,17 @@ are unchanged from today's behavior in every combination. ## Dependencies & Coordination -- **Shared query with item 1 (separate spec, out of scope here)**: A separate, parallel effort - (issue #37148 item 1) is reworking the same database candidate-scan query this fix's chunk loop - currently calls repeatedly, to fix an unrelated planner-instability problem on very large - folders. This fix reduces how often that query is invoked for the field-filter-only case - addressed here, which lowers this case's exposure to item 1's problem, but does not fix it. Item - 1's eventual change to that query must continue to behave correctly when invoked at most once - per request, as this fix will do for its case. Coordinate before either change merges independently. +- **Shared query with item 1 (separate spec, out of scope here) — hard dependency for SC-002.** + A separate, parallel effort (issue #37148 item 1, spec'd in #37230) is reworking the same + database candidate-scan query this fix's chunk loop currently calls repeatedly, to fix an + unrelated planner-instability problem on very large folders. This fix reduces how often that + query is invoked for the field-filter-only case addressed here (from ~4 to 1), which is a real + win regardless of #37230's status — but SC-002's search-comparable latency target additionally + requires that single remaining scan itself to be fast, which is exactly what #37230 delivers. + In other words: FR-002 (scan count) can ship and be verified independently of #37230; SC-002 + (latency target) cannot. Item 1's eventual change to that query must continue to behave + correctly when invoked at most once per request, as this fix will do for its case. Coordinate + before either change merges independently, and sequence SC-002's validation after #37230 lands. ## Assumptions @@ -197,3 +221,7 @@ are unchanged from today's behavior in every combination. - The existing per-field index query logic (already used by the current hybrid strategy) is assumed to be reusable as-is for the single-pass case; no new field-to-index translation logic is expected to be needed. +- Test-first development (Constitution Principle V) applies: expected home for these tests is + `BrowserAPITest` (integration — scan-count assertions for FR-002, result-set parity for + FR-005/FR-007/FR-008) plus a dedicated timing assertion for SC-002 that is only meaningful + once #37230 lands — exact test design deferred to the planning phase. From ec1dcfb9113edef3d3ba73ae5583c1ea17aef2a0 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Tue, 1 Sep 2026 20:03:36 -0300 Subject: [PATCH 3/7] docs(content-drive): correct FR-009 reasoning and bound both scan and ES round trips in FR-002 --- .../spec.md | 116 ++++++++++++------ 1 file changed, 76 insertions(+), 40 deletions(-) diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md index c4841ebfba19..797822d8629b 100644 --- a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md +++ b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md @@ -19,25 +19,31 @@ A content author or administrator browsing a folder with thousands of items applies a filter on a content-type field (for example, a Text, Select, Date-range, Multi-select, or Category field). Today, once the filter is entirely resolvable against the search index, the system still repeats -its full database scan of the folder's contents multiple times before it has gathered enough -matches to fill a page — on a ~20,000-item folder this took roughly four repeats and made the -response six times slower than an equivalent search. The user should instead get their filtered -results back in roughly the time a comparable search takes, without repeated re-scanning. +its full database scan of the folder's contents — and, per matching chunk, a separate +Elasticsearch filtering call — before it has gathered enough matches to fill a page. On a +~20,000-item folder with sparse matches this can iterate up to ~23 times (chunk size 900); the +issue's own reference case measured roughly four repeats and a six-fold slowdown versus an +equivalent search, but that reference case is a specific measured point, not the worst case (a +folder with even sparser matches or a larger size iterates more). The user should instead get +their filtered results back in roughly the time a comparable search takes, without repeated +re-scanning or repeated ES round trips. **Why this priority**: This is the dominant remaining performance gap Content Drive has against its baselines once the highest-cost item (item 1) is addressed; field filtering over large folders is a normal, frequent authoring action. **Independent Test**: Can be fully tested by applying a single content-type field filter (no Tag, -no Relationship, no workflow, no free-text term) against a folder with a large number of children -and confirming both that the number of times the underlying candidate scan runs drops to at most -one, and that the returned result set is unchanged from today's behavior. +no Relationship, no workflow, no free-text term) against a folder with a large number of sparse +matches and confirming both that the number of times the underlying candidate scan runs AND the +number of Elasticsearch filtering calls each drop to at most one, and that the returned result +set is unchanged from today's behavior. **Acceptance Scenarios**: -1. **Given** a folder with ~20,000 children and a filter on a Text field that matches 40 items, - **When** the user applies the filter, **Then** the system scans the folder's candidate content - at most once (not four times as today) and returns the same 40 matching items. +1. **Given** a folder with ~20,000 children and a filter on a Text field that matches 40 sparse + items, **When** the user applies the filter, **Then** the system scans the folder's candidate + content at most once and issues at most one Elasticsearch filtering call (not up to ~23 of + each as today's sparse worst case) and returns the same 40 matching items. 2. **Given** the same folder and filter, **When** the user applies a Date-range, Multi-select, or Category filter instead, **Then** results are identical to today's behavior and the same single-scan improvement applies. @@ -96,19 +102,35 @@ are unchanged from today's behavior in every combination. - **FR-001**: System MUST determine, for each field-filter search request, whether every requested field criterion is resolvable purely against the search index (i.e., no criterion requires the database to preserve immediate visibility of recent writes). -- **FR-002 — implementation named (2026-08-31, per review): hybrid single-scan, not - index-only.** When that condition holds, and the request has no workflow filter and no - free-text or file-name term, System MUST resolve the request without repeatedly re-scanning - the database candidate set — the underlying database scan MUST execute at most once per - request, regardless of folder size or how sparse the matches are. This is a **hybrid** fix: - the existing database-first candidate scan still runs (fixing only its repeat-execution - count from ~4 down to 1), it does not switch to `buildPureESQuery`/index-only resolution. - That alternative was considered and rejected: `buildPureESQuery` (`BrowserAPIImpl.java:605`) - has no folder/parentPath filter today (only host-level scoping), and moving structural - filtering there would conflict with this spec's own cited ADR-0018 requirement that - structural/metadata filtering stay database-first always (see Legacy Considerations). FR-003, - FR-004, and FR-007 below describe *preserving* existing database-sourced behavior, not - building new index-side folder/permission/pagination logic. +- **FR-002 — implementation named (2026-08-31) and mechanism corrected (2026-09-01, per + review): hybrid single-pass, bounding both DB scans and ES round trips as one criterion.** + When that condition holds, and the request has no workflow filter and no free-text or + file-name term, System MUST resolve the request in a **single pass** over the folder's + candidate content: at most one database scan AND at most one Elasticsearch filtering call for + the whole request, regardless of folder size or how sparse the matches are. This is a + **hybrid** fix: the existing database-first candidate scan still runs, it does not switch to + `buildPureESQuery`/index-only resolution. That alternative was considered and rejected: + `buildPureESQuery` (`BrowserAPIImpl.java:605`) has no folder/parentPath filter today (only + host-level scoping), and moving structural filtering there would conflict with this spec's own + cited ADR-0018 requirement that structural/metadata filtering stay database-first always (see + Legacy Considerations). FR-003, FR-004, and FR-007 below describe *preserving* existing + database-sourced behavior, not building new index-side folder/permission/pagination logic. + **Correction to the mechanism, and why both round trips need one bound**: + `getContentByChunks` (the loop this fix changes) already has an early exit — it stops as soon + as `accumulatedContent.size() >= maxRows` (`BrowserAPIImpl.java:291`) or the DB chunk comes + back partial (`:298`). The previously-observed "~4 repeats" is the **sparse-match worst + case**, not today's universal behavior — a folder with dense matches can already exit after + chunk 1. What the early exit does *not* bound is the **ES side**: each loop iteration calls + `processESDirectly` once per chunk (`BrowserAPIImpl.java:343-344`), so a sparse-match, + 20,000-item folder at the default `BROWSER_CONTENT_CHUNK_SIZE` (900, + `BrowserAPIImpl.java:547`) can still iterate up to `20,000 / 900 ≈ 23` times before either + exiting or hitting `BROWSER_DB_MAX_SCAN_ROWS` (default 50,000, `:734-735`) — 23 DB round trips + *and* 23 ES round trips, not the "~4" the issue measured on its specific reference case. A fix + that only forces the DB scan itself into one query but leaves the surrounding chunked-ES-call + loop in place would not close this gap. FR-002 is therefore one bound covering both: the + database candidate scan for this case MUST assemble the full candidate set in a single query + (no chunking), and Elasticsearch filtering over that candidate set MUST run as a single call, + not one call per artificial chunk. - **FR-003**: The single-pass resolution in FR-002 MUST still be scoped to the folder (and site) the user is browsing — it must never return content from outside the requested folder. - **FR-004**: System MUST still apply the mandatory read-permission filter, sourced from the @@ -126,18 +148,28 @@ are unchanged from today's behavior in every combination. pagination behavior). - **FR-008**: Combining a content-type field filter with a Tag filter MUST continue to return the same correct results as today. -- **FR-009 — RESOLVED (2026-08-24, correction 2026-08-31): no dedicated kill switch, and no - existing config already covers this.** No independently-toggleable operator flag for the - single-pass behavior in FR-002. Decision reasoning: the freshness trade-off it would guard - (see Assumptions) is narrow and scoped — it applies only when zero database-required criteria - are present — and mirrors a trade-off ADR-0018 already accepts by default for free-text - search, without a dedicated flag for that case either. Correction: `BROWSE_API_HEURISTIC_TYPE` - (`SearchHeuristicType`, `BrowserAPIImpl.java:465-470`) does **not** apply here — it toggles - between `HYBRID_SINGLE_CHUNKED_QUERY_ES` and `PURE_ES` for the free-text-filtering code path - (`doElasticSearchTextFiltering`) only, and has no effect on `getContentByChunks`'s repeat-scan - loop, which is what FR-002 changes. There is genuinely no existing config-level escape hatch - for this fix; if the single-pass path ever needs disabling, that would require a new flag - added at that time, not one that exists today. +- **FR-009 — RESOLVED (2026-08-24): no dedicated kill switch. Reasoning corrected + (2026-09-01, per review) — `BROWSE_API_HEURISTIC_TYPE` DOES apply here, and it is not a safe + substitute.** No independently-toggleable operator flag for the single-pass behavior in + FR-002. Decision reasoning: the freshness trade-off it would guard (see Assumptions) is narrow + and scoped — it applies only when zero database-required criteria are present — and mirrors a + trade-off ADR-0018 already accepts by default for free-text search, without a dedicated flag + for that case either. **Correction**: `BROWSE_API_HEURISTIC_TYPE` (`SearchHeuristicType`, + `BrowserAPIImpl.java:465-470`) is **not** limited to free-text filtering — despite its name, + `doElasticSearchTextFiltering` is the dispatcher for every request `isUseElasticSearchForFiltering` + routes to ES, and that includes a pure field-filter request with **zero** free-text/file-name + term, as long as it has at least one index-routed field criterion + (`isUseElasticSearchForFiltering`, `BrowserAPIImpl.java:1577-1584`: `hasIndexFieldCriteria` + alone is sufficient). So this config *does* gate the FR-002 case. It is still not usable as a + kill switch, though: setting it to `PURE_ES` is not inert for this case — `doPureESQuery`'s + guard explicitly throws `DotRuntimeException` for any request with field criteria present + (`BrowserAPIImpl.java:496-504`: *"Content Drive field filters (userSearchable) are not + supported under the PURE_ES heuristic"*), i.e. it would break every field-filter request, not + disable just the single-pass optimization. There is genuinely no existing config-level escape + hatch for *this specific fix*; if the single-pass path ever needs disabling, that requires a + new flag added at that time. The spec previously stated the opposite conclusion for the wrong + reason (claiming the config didn't apply at all) — corrected here so an operator doesn't reach + for `BROWSE_API_HEURISTIC_TYPE=PURE_ES` expecting it to be a safe no-op. ### Key Entities @@ -154,9 +186,12 @@ are unchanged from today's behavior in every combination. ### Measurable Outcomes -- **SC-001**: For a field filter with no database-required criteria over a folder with roughly - 20,000 items, the number of times the system re-scans the folder's candidate content drops from - four to at most one. +- **SC-001 — corrected (2026-09-01, per review) to bound both round-trip types.** For a field + filter with no database-required criteria over a folder with roughly 20,000 items and sparse + matches (the worst case, not the previously-cited "four" reference point — dense-match cases + can already exit early today), the number of database scans drops to **at most one** AND the + number of Elasticsearch filtering calls drops to **at most one**, down from up to ~23 of each + at the default chunk size on this folder size (see FR-002). - **SC-002**: Response time for that same case falls to within 20% of the response time of the closest equivalent content-search operation (matching the threshold already used to evaluate Content Drive elsewhere in this investigation). **Dependency**: because FR-002 is a @@ -195,8 +230,9 @@ are unchanged from today's behavior in every combination. A separate, parallel effort (issue #37148 item 1, spec'd in #37230) is reworking the same database candidate-scan query this fix's chunk loop currently calls repeatedly, to fix an unrelated planner-instability problem on very large folders. This fix reduces how often that - query is invoked for the field-filter-only case addressed here (from ~4 to 1), which is a real - win regardless of #37230's status — but SC-002's search-comparable latency target additionally + query is invoked for the field-filter-only case addressed here (from up to ~23 in the + sparse-match worst case to 1), which is a real win regardless of #37230's status — but SC-002's + search-comparable latency target additionally requires that single remaining scan itself to be fast, which is exactly what #37230 delivers. In other words: FR-002 (scan count) can ship and be verified independently of #37230; SC-002 (latency target) cannot. Item 1's eventual change to that query must continue to behave From b2cfdffa91df89c1e0152631474aa7e2c9007079 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Tue, 1 Sep 2026 22:41:14 -0300 Subject: [PATCH 4/7] docs(content-drive): add Phase 1 plan artifacts and resolve SC-001 ES-call metric --- .specify/feature.json | 2 +- .../data-model.md | 61 +++++++++++++++++++ .../spec.md | 20 ++++-- 3 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md diff --git a/.specify/feature.json b/.specify/feature.json index af3a6bf46a7e..8dec653dab5c 100644 --- a/.specify/feature.json +++ b/.specify/feature.json @@ -1 +1 @@ -{"feature_directory":"specs/36950-remove-dead-core-web-libs"} +{"feature_directory":"specs/37184-content-drive-field-filter-chunk-multiplier"} diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md b/specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md new file mode 100644 index 000000000000..22878b895967 --- /dev/null +++ b/specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md @@ -0,0 +1,61 @@ +# Phase 1 Data Model: Content Drive Field-Filter Chunk Multiplier + +This fix introduces **no new persistent entity, DB table/column, or index mapping**. It +changes control flow (how many times an existing query/call runs) inside +`com.dotcms.browser.BrowserAPIImpl`, over data shapes that already exist. This document +records the existing in-memory entities/fields the fix reads or reasons about, since the +spec's Key Entities section names them at a conceptual level and the plan needs to tie them +to concrete code. + +## Field-filter search request (`BrowserQuery`) + +Existing class (`dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java`), unchanged by +this fix. Fields relevant to the single-pass condition (FR-002): + +| Field | Type | Relevance to this fix | +|-------|------|------------------------| +| `fieldCriteria` | `List` | Source of the DB-vs-index routing bucket per criterion; single-pass requires none of these to be `RoutingBucket.DB`. | +| `workflowSchemeIds` | `Set` | Single-pass requires this to be empty (workflow filters must stay on the existing chunked path per FR-005). | +| `workflowStepIds` | `Set` | Same as above. | +| `filter` | `String` | Free-text term; single-pass requires this unset (`!UtilMethods.isSet(...)`). | +| `fileName` | `String` | File-name term; same requirement as `filter`. | +| `useElasticsearchFiltering` | `boolean` | Must already be `true` for the request to reach `doElasticSearchTextFiltering` at all — precondition, not part of the new check. | +| `contentCursor` | `int` | DB row offset for the current page; read and advanced identically to today — pagination semantics are not changed by this fix (see research.md R3). | + +No new field is added to `BrowserQuery`. See research.md R2 for why the single-pass +condition is computed from existing fields rather than a new precomputed flag. + +## Field criterion (`FieldSearchCriteria`) + +Existing class (`dotCMS/src/main/java/com/dotcms/browser/FieldSearchCriteria.java`), +unchanged by this fix. + +| Field | Type | Relevance to this fix | +|-------|------|------------------------| +| `bucket` | `RoutingBucket` (`DB` \| `INDEX`) | The single fact this fix's new condition reads per criterion — already assigned upstream per ADR-0018's routing table (Tag/Relationship → `DB`; Text/Date/Multi-select/Category → `INDEX`). | +| `field` | `Field` (content-type field model) | Used elsewhere (e.g. `instanceof TagField`/`RelationshipField` checks) — not newly consumed by this fix beyond what `bucket` already summarizes. | +| `values` / `from`/`to` (per `FilterKind`) | `List` / `String` | Unchanged; carried through to the (unchanged) ES query construction for index-bucket criteria. | + +## Candidate content set (in-memory, not persisted) + +The list of DB-ordered candidate inodes produced by `selectQuery(...)` + a `DotConnect` +fetch. Today assembled incrementally across chunk iterations +(`accumulatedContent` in `getContentByChunks`); after this fix, for the single-pass case, +assembled from a single fetch bounded by `BROWSER_DB_MAX_SCAN_ROWS` (existing config, +default 50,000) instead of `BROWSER_CONTENT_CHUNK_SIZE` (existing config, default 900) +per-iteration slices. No new state is introduced — this is a change to how many round trips +populate the same in-memory list. + +## Elasticsearch filtering call (`processESDirectly`) + +Existing package-private method; unchanged internally. Consumed differently: today invoked +once per DB chunk (up to ~23 times for the spec's worst case); after this fix, invoked once +per request for the single-pass case, with the full candidate set. Internally it still may +split into multiple physical ES queries above `calculateMaxInodesPerESQuery` (~900 inodes) — +see research.md R4 and plan.md Complexity Tracking Q1 for the open question this raises for +the SC-001 metric. + +## State transitions + +None. This is a stateless, per-request control-flow change — no entity moves between states +as part of this fix. diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md index 797822d8629b..bbd4d08ee4dd 100644 --- a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md +++ b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md @@ -186,12 +186,20 @@ are unchanged from today's behavior in every combination. ### Measurable Outcomes -- **SC-001 — corrected (2026-09-01, per review) to bound both round-trip types.** For a field - filter with no database-required criteria over a folder with roughly 20,000 items and sparse - matches (the worst case, not the previously-cited "four" reference point — dense-match cases - can already exit early today), the number of database scans drops to **at most one** AND the - number of Elasticsearch filtering calls drops to **at most one**, down from up to ~23 of each - at the default chunk size on this folder size (see FR-002). +- **SC-001 — corrected (2026-09-01, per review) to bound both round-trip types; unit of + measure resolved (2026-09-01, during planning).** For a field filter with no database-required + criteria over a folder with roughly 20,000 items and sparse matches (the worst case, not the + previously-cited "four" reference point — dense-match cases can already exit early today), the + number of database scans drops to **at most one** AND the number of **logical** Elasticsearch + filtering calls (invocations of `processESDirectly`) drops to **at most one**, down from up to + ~23 of each at the default chunk size on this folder size (see FR-002). **Resolved + clarification**: "Elasticsearch filtering calls" counts logical invocations of the filtering + step, not physical ES HTTP round trips. `processESDirectly` already internally re-splits any + candidate set above ~900 inodes into multiple physical ES queries (Lucene's 1024-boolean-clause + limit) — this fix does not change that internal splitting, and a 20,000-candidate worst case + can still produce ~20-23 physical ES round trips after this fix ships. Bounding the physical + round-trip count too would require switching the query mechanism itself (inode enumeration → + a `terms` filter), which is a separate, larger change and explicitly out of scope here. - **SC-002**: Response time for that same case falls to within 20% of the response time of the closest equivalent content-search operation (matching the threshold already used to evaluate Content Drive elsewhere in this investigation). **Dependency**: because FR-002 is a From d06fed6c34b6cdda5eab192f3dc24c9a2aaa3742 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Wed, 2 Sep 2026 14:59:33 -0300 Subject: [PATCH 5/7] docs(content-drive): clarify ES-filter-before-permission ordering predates this fix --- .../spec.md | 34 ++++++++++++------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md index bbd4d08ee4dd..6efebcbf86bb 100644 --- a/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md +++ b/specs/37184-content-drive-field-filter-chunk-multiplier/spec.md @@ -151,10 +151,15 @@ are unchanged from today's behavior in every combination. - **FR-009 — RESOLVED (2026-08-24): no dedicated kill switch. Reasoning corrected (2026-09-01, per review) — `BROWSE_API_HEURISTIC_TYPE` DOES apply here, and it is not a safe substitute.** No independently-toggleable operator flag for the single-pass behavior in - FR-002. Decision reasoning: the freshness trade-off it would guard (see Assumptions) is narrow - and scoped — it applies only when zero database-required criteria are present — and mirrors a - trade-off ADR-0018 already accepts by default for free-text search, without a dedicated flag - for that case either. **Correction**: `BROWSE_API_HEURISTIC_TYPE` (`SearchHeuristicType`, + FR-002. Decision reasoning, simplified (2026-09-01, per review): there is nothing new for a + kill switch to guard. The index-lag behavior once described in Assumptions as "a deliberate, + scoped trade-off for this one case" already exists today in the current hybrid-chunked path — + `getChunkFiltered` (`BrowserAPIImpl.java:342-346`) runs `processESDirectly` before the + permission filter on every chunk today, so a just-written, not-yet-indexed item is already + excluded from field-filter results before this fix ships. FR-002 changes how many times that + loop runs, not whether this behavior exists — so a flag to disable "the freshness trade-off + this fix introduces" would have nothing to disable; the trade-off isn't this fix's to concede. + **Correction**: `BROWSE_API_HEURISTIC_TYPE` (`SearchHeuristicType`, `BrowserAPIImpl.java:465-470`) is **not** limited to free-text filtering — despite its name, `doElasticSearchTextFiltering` is the dispatcher for every request `isUseElasticSearchForFiltering` routes to ES, and that includes a pure field-filter request with **zero** free-text/file-name @@ -253,15 +258,18 @@ are unchanged from today's behavior in every combination. Relationship field (the two field types the codebase already resolves against the database to preserve immediate visibility of recent writes); this mirrors the existing, documented routing rule and is not a new judgment call introduced by this fix. -- **RESOLVED (2026-08-24), no product sign-off required.** Accepting a brief, index-lag-bounded - delay before a just-written item appears in a field-filter-only search (no Tag, no - Relationship, no workflow, no free-text term) is a deliberate, scoped trade-off for this one - case, mirroring the trade-off ADR-0018 already accepts by default for free-text search — it is - not a general relaxation of Content Drive's read-your-writes guarantee, which continues to hold - for every other filter combination. Treated as a technical decision, not a product one, because - it extends an already-accepted architectural trade-off to one additional, narrowly-scoped case - rather than introducing a new one. Ships as default behavior (see FR-009 — no dedicated kill - switch either). +- **RESOLVED (2026-08-24); rescoped (2026-09-01, per review) — this is pre-existing behavior, + not a trade-off this fix introduces.** A brief, index-lag-bounded delay before a just-written + item appears in a field-filter-only search (no Tag, no Relationship, no workflow, no free-text + term) already exists **today**, before this fix: `getChunkFiltered` + (`BrowserAPIImpl.java:342-346`) runs `processESDirectly` before the permission filter on every + chunk of the current hybrid-chunked path, so a not-yet-indexed item is already excluded from + results now. This fix (FR-002) changes how many times that loop runs, not whether ES-index-lag + gates field-filter results — so there is no new trade-off here for product sign-off to weigh + in on; the behavior predates this item and continues unchanged for every filter combination + that already goes through ES filtering. No product sign-off needed, because there is nothing + new being decided. No dedicated kill switch either (see FR-009) — not because the trade-off is + "narrow and scoped," but because there is nothing this fix introduces for a switch to disable. - The existing per-field index query logic (already used by the current hybrid strategy) is assumed to be reusable as-is for the single-pass case; no new field-to-index translation logic is expected to be needed. From c004ffeff3b1bcae28ca2e862802c62ed77e84f0 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Wed, 2 Sep 2026 15:54:53 -0300 Subject: [PATCH 6/7] chore(speckit): stop tracking .specify/feature.json --- .gitignore | 5 +++++ .specify/feature.json | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) delete mode 100644 .specify/feature.json diff --git a/.gitignore b/.gitignore index 9a1c952d10da..41e94f07dd73 100644 --- a/.gitignore +++ b/.gitignore @@ -231,3 +231,8 @@ specs/*/research.md specs/*/tasks.md specs/*/quickstart.md specs/*/checklists/ + +# Per-developer pointer to the feature you are currently working on, rewritten by +# /speckit-specify. Committing it would aim everyone else's /speckit-plan at your +# feature, and every concurrent feature branch would collide on its single line. +.specify/feature.json diff --git a/.specify/feature.json b/.specify/feature.json deleted file mode 100644 index 8dec653dab5c..000000000000 --- a/.specify/feature.json +++ /dev/null @@ -1 +0,0 @@ -{"feature_directory":"specs/37184-content-drive-field-filter-chunk-multiplier"} From fb6258ca7e8f77dbd8acefc8af677df6caf5c130 Mon Sep 17 00:00:00 2001 From: ihoffmann-dot Date: Wed, 2 Sep 2026 16:55:56 -0300 Subject: [PATCH 7/7] chore(content-drive): remove Phase 1 plan artifacts from spec PR, per Spec-Kit flow --- .../data-model.md | 61 ------------------- 1 file changed, 61 deletions(-) delete mode 100644 specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md diff --git a/specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md b/specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md deleted file mode 100644 index 22878b895967..000000000000 --- a/specs/37184-content-drive-field-filter-chunk-multiplier/data-model.md +++ /dev/null @@ -1,61 +0,0 @@ -# Phase 1 Data Model: Content Drive Field-Filter Chunk Multiplier - -This fix introduces **no new persistent entity, DB table/column, or index mapping**. It -changes control flow (how many times an existing query/call runs) inside -`com.dotcms.browser.BrowserAPIImpl`, over data shapes that already exist. This document -records the existing in-memory entities/fields the fix reads or reasons about, since the -spec's Key Entities section names them at a conceptual level and the plan needs to tie them -to concrete code. - -## Field-filter search request (`BrowserQuery`) - -Existing class (`dotCMS/src/main/java/com/dotcms/browser/BrowserQuery.java`), unchanged by -this fix. Fields relevant to the single-pass condition (FR-002): - -| Field | Type | Relevance to this fix | -|-------|------|------------------------| -| `fieldCriteria` | `List` | Source of the DB-vs-index routing bucket per criterion; single-pass requires none of these to be `RoutingBucket.DB`. | -| `workflowSchemeIds` | `Set` | Single-pass requires this to be empty (workflow filters must stay on the existing chunked path per FR-005). | -| `workflowStepIds` | `Set` | Same as above. | -| `filter` | `String` | Free-text term; single-pass requires this unset (`!UtilMethods.isSet(...)`). | -| `fileName` | `String` | File-name term; same requirement as `filter`. | -| `useElasticsearchFiltering` | `boolean` | Must already be `true` for the request to reach `doElasticSearchTextFiltering` at all — precondition, not part of the new check. | -| `contentCursor` | `int` | DB row offset for the current page; read and advanced identically to today — pagination semantics are not changed by this fix (see research.md R3). | - -No new field is added to `BrowserQuery`. See research.md R2 for why the single-pass -condition is computed from existing fields rather than a new precomputed flag. - -## Field criterion (`FieldSearchCriteria`) - -Existing class (`dotCMS/src/main/java/com/dotcms/browser/FieldSearchCriteria.java`), -unchanged by this fix. - -| Field | Type | Relevance to this fix | -|-------|------|------------------------| -| `bucket` | `RoutingBucket` (`DB` \| `INDEX`) | The single fact this fix's new condition reads per criterion — already assigned upstream per ADR-0018's routing table (Tag/Relationship → `DB`; Text/Date/Multi-select/Category → `INDEX`). | -| `field` | `Field` (content-type field model) | Used elsewhere (e.g. `instanceof TagField`/`RelationshipField` checks) — not newly consumed by this fix beyond what `bucket` already summarizes. | -| `values` / `from`/`to` (per `FilterKind`) | `List` / `String` | Unchanged; carried through to the (unchanged) ES query construction for index-bucket criteria. | - -## Candidate content set (in-memory, not persisted) - -The list of DB-ordered candidate inodes produced by `selectQuery(...)` + a `DotConnect` -fetch. Today assembled incrementally across chunk iterations -(`accumulatedContent` in `getContentByChunks`); after this fix, for the single-pass case, -assembled from a single fetch bounded by `BROWSER_DB_MAX_SCAN_ROWS` (existing config, -default 50,000) instead of `BROWSER_CONTENT_CHUNK_SIZE` (existing config, default 900) -per-iteration slices. No new state is introduced — this is a change to how many round trips -populate the same in-memory list. - -## Elasticsearch filtering call (`processESDirectly`) - -Existing package-private method; unchanged internally. Consumed differently: today invoked -once per DB chunk (up to ~23 times for the spec's worst case); after this fix, invoked once -per request for the single-pass case, with the full candidate set. Internally it still may -split into multiple physical ES queries above `calculateMaxInodesPerESQuery` (~900 inodes) — -see research.md R4 and plan.md Complexity Tracking Q1 for the open question this raises for -the SC-001 metric. - -## State transitions - -None. This is a stateless, per-request control-flow change — no entity moves between states -as part of this fix.