OAK-12348: Property and node type indexes support costPerEntry/costPerExecution overrides - #3076
Open
bhabegger wants to merge 1 commit into
Open
OAK-12348: Property and node type indexes support costPerEntry/costPerExecution overrides#3076bhabegger wants to merge 1 commit into
bhabegger wants to merge 1 commit into
Conversation
…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
force-pushed
the
issue/OAK-12348
branch
from
August 12, 2026 08:56
80a873b to
1893ff0
Compare
bhabegger
marked this pull request as ready for review
August 12, 2026 14:57
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.
What
PropertyIndex/NodeTypeIndex(oak-core) compute query cost purely fromindexed 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 readcostPerEntry/costPerExecution.In production this let the
nodeType/property index win a cost comparisonagainst 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:Groupqueries against atagged
authorizablesLucene index). The workaround each time was amanual
costPerEntry/costPerExecutionoverride — but that only exists onthe Lucene/Elastic side.
How
PropertyIndexPlanandPropertyIndexLookupeach 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 namesoak-search'sFulltextIndexConstantsalready uses for Lucene/Elastic).
getCost(...)— dispatches between the two based onFT_OAK-12348(
PropertyIndexLookup.FT_OAK_12348_ENABLE), enabled by default: withno properties set,
getCostConfigurable()reproducesgetCostLegacy()exactly, so this is behavior-preserving for every existing index
definition — the toggle is an escape hatch, not an opt-in gate.
NodeTypeIndexneeds no code changes at all — its cost is the sum of twoPropertyIndexLookup.getCost()calls (jcr:primaryType,jcr:mixinTypes),so it picks up the override transitively (covered by a dedicated test).
IndexUtilsgains a smallgetOptionalValue(NodeState, String, double)helper (mirrors
oak-search'sIndexDefinition.getOptionalValue, whichoak-corecan't depend on directly) instead of duplicating the sameproperty read in both classes.
Out of scope:
resultCacheSize(mentioned in the originating request) is aLucene/Elastic query-result caching concept with no equivalent in
PropertyIndex'sContentMirrorStoreStrategy-based lookup.Testing
New/updated tests in
PropertyIndexTest,PropertyIndexLookupTest(unchanged,still green), and
NodeTypeIndexTestcover: default behavior unchangedregardless of toggle position, override taking effect by default, the
toggle correctly falling back to the legacy formula when disabled, the
costPerEntry=0infinity-guard (must not becomeNaN), the unique-indexshort-circuit surviving an override, and
NodeTypeIndexpicking up theoverride with zero code changes in its own package.
Full
oak-coresuite: 4779 tests, 0 failures.