bhabegger opened a new pull request, #3076:
URL: https://github.com/apache/jackrabbit-oak/pull/3076
## What
`PropertyIndex`/`NodeTypeIndex` (`oak-core`) compute query cost purely from
indexed entry counts plus a fixed overhead of 2, with no way to influence
the estimate from the index definition — unlike Lucene/Elastic indexes
(`oak-search`), which already read `costPerEntry`/`costPerExecution`.
In production this let the `nodeType`/property index win a cost comparison
against a more selective, purpose-built index for the same query, because
the built-in entry-count estimate can be significantly wrong at scale (seen
in three related incidents on `rep:User`/`rep:Group` queries against a
tagged `authorizables` Lucene index). The workaround each time was a
manual `costPerEntry`/`costPerExecution` override — but that only exists on
the Lucene/Elastic side.
## How
`PropertyIndexPlan` and `PropertyIndexLookup` each split into:
- `getCostLegacy(...)` — the original hardcoded formula, unconditionally.
- `getCostConfigurable(...)` — `cost = costPerExecution + costPerEntry *
entryCount`,
both optionally set on the property index definition (new
`IndexConstants.COST_PER_ENTRY`/
`COST_PER_EXECUTION`, same property names `oak-search`'s
`FulltextIndexConstants`
already uses for Lucene/Elastic).
- `getCost(...)` — dispatches between the two based on `FT_OAK-12348`
(`PropertyIndexLookup.FT_OAK_12348_ENABLE`), **enabled by default**: with
no properties set, `getCostConfigurable()` reproduces `getCostLegacy()`
exactly, so this is behavior-preserving for every existing index
definition — the toggle is an escape hatch, not an opt-in gate.
`NodeTypeIndex` needs no code changes at all — its cost is the sum of two
`PropertyIndexLookup.getCost()` calls (`jcr:primaryType`, `jcr:mixinTypes`),
so it picks up the override transitively (covered by a dedicated test).
`IndexUtils` gains a small `getOptionalValue(NodeState, String, double)`
helper (mirrors `oak-search`'s `IndexDefinition.getOptionalValue`, which
`oak-core` can't depend on directly) instead of duplicating the same
property read in both classes.
Out of scope: `resultCacheSize` (mentioned in the originating request) is a
Lucene/Elastic query-result caching concept with no equivalent in
`PropertyIndex`'s `ContentMirrorStoreStrategy`-based lookup.
## Testing
New/updated tests in `PropertyIndexTest`, `PropertyIndexLookupTest`
(unchanged,
still green), and `NodeTypeIndexTest` cover: default behavior unchanged
regardless of toggle position, override taking effect by default, the
toggle correctly falling back to the legacy formula when disabled, the
`costPerEntry=0` infinity-guard (must not become `NaN`), the unique-index
short-circuit surviving an override, and `NodeTypeIndex` picking up the
override with zero code changes in its own package.
Full `oak-core` suite: 4779 tests, 0 failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
