Skip to content

OAK-12348: Property and node type indexes support costPerEntry/costPerExecution overrides - #3076

Open
bhabegger wants to merge 1 commit into
apache:trunkfrom
bhabegger:issue/OAK-12348
Open

OAK-12348: Property and node type indexes support costPerEntry/costPerExecution overrides#3076
bhabegger wants to merge 1 commit into
apache:trunkfrom
bhabegger:issue/OAK-12348

Conversation

@bhabegger

Copy link
Copy Markdown
Contributor

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.

…rExecution overrides

PropertyIndex/NodeTypeIndex (oak-core) computed cost purely from indexed
entry counts, with a fixed overhead of 2, and no way to influence the
estimate from the index definition -- unlike Lucene/Elastic indexes
(oak-search), which already read costPerEntry/costPerExecution.

In production this caused the nodeType/property index to win cost
comparisons against a more selective, purpose-built index for the same
query (three related incidents), because the built-in entry-count
estimate can be significantly wrong at scale.

PropertyIndexPlan and PropertyIndexLookup now each split into
getCostLegacy() (the original hardcoded formula), getCostConfigurable()
(cost = costPerExecution + costPerEntry * entryCount, both optionally
set on the property index definition), and getCost() which dispatches
between them based on FT_OAK-12348 (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
changes at all -- its cost is the sum of two PropertyIndexLookup.getCost()
calls (jcr:primaryType, jcr:mixinTypes), so it picks up the override
transitively.

IndexUtils gains a small public getOptionalValue(NodeState, String,
double) helper (mirroring oak-search's IndexDefinition.getOptionalValue,
which oak-core cannot depend on directly) used by both getCostConfigurable
methods instead of duplicating the same property read twice.
@bhabegger
bhabegger marked this pull request as ready for review August 12, 2026 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant