thomasmueller commented on code in PR #3076:
URL: https://github.com/apache/jackrabbit-oak/pull/3076#discussion_r4063032486
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java:
##########
@@ -135,18 +144,63 @@ Set<IndexStoreStrategy> getStrategies(NodeState
definition) {
definition, INDEX_CONTENT_NODE_NAME);
}
+ /**
+ * Dispatches to {@link #getCostConfigurable} or {@link #getCostLegacy}
+ * based on {@code disableConfigurableCosts}.
+ */
public double getCost(Filter filter, String propertyName, PropertyValue
value) {
+ return disableConfigurableCosts
+ ? getCostLegacy(filter, propertyName, value)
+ : getCostConfigurable(filter, propertyName, value);
+ }
+
+ /**
+ * Original cost formula: {@code COST_OVERHEAD + entryCount}. Ignores
+ * {@code costPerEntry}/{@code costPerExecution} even if set on the index
+ * definition.
+ */
+ public double getCostLegacy(Filter filter, String propertyName,
PropertyValue value) {
+ NodeState indexMeta = getIndexNode(root, propertyName, filter);
+ if (indexMeta == null) {
+ return Double.POSITIVE_INFINITY;
+ }
+ return getCost(indexMeta, filter, value, COST_OVERHEAD, 1.0);
+ }
+
+ /**
+ * {@code cost = costPerExecution + costPerEntry * entryCount}, both
+ * optionally configured on the index definition (OAK-12348). Defaults
+ * ({@code costPerEntry=1.0}, {@code costPerExecution=COST_OVERHEAD})
+ * reproduce {@link #getCostLegacy} exactly.
+ */
+ public double getCostConfigurable(Filter filter, String propertyName,
PropertyValue value) {
NodeState indexMeta = getIndexNode(root, propertyName, filter);
if (indexMeta == null) {
return Double.POSITIVE_INFINITY;
}
+ double costPerEntry = IndexUtils.getOptionalValue(indexMeta,
IndexConstants.COST_PER_ENTRY, 1.0);
+ double costPerExecution = IndexUtils.getOptionalValue(indexMeta,
IndexConstants.COST_PER_EXECUTION, COST_OVERHEAD);
+ return getCost(indexMeta, filter, value, costPerExecution,
costPerEntry);
+ }
+
+ // Shared by getCostLegacy/getCostConfigurable, which only differ in
+ // costPerExecution/costPerEntry -- legacy hardcodes them, configurable
+ // reads them from indexMeta (which is why they can't be precomputed
+ // earlier than this: they depend on which property's index definition
+ // this call resolved, and the same PropertyIndexLookup is reused across
+ // different property names, e.g. by NodeTypeIndexLookup).
+ private double getCost(NodeState indexMeta, Filter filter, PropertyValue
value,
+ double costPerExecution, double costPerEntry) {
Set<IndexStoreStrategy> strategies = getStrategies(indexMeta);
+ if (strategies.isEmpty()) {
+ return MAX_COST;
+ }
ValuePattern pattern = new ValuePattern(indexMeta);
- double cost = strategies.isEmpty() ? MAX_COST : COST_OVERHEAD;
+ double entryCount = 0;
Review Comment:
```suggestion
long entryCount = 0;
```
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java:
##########
@@ -205,6 +200,30 @@ String getName() {
return cost;
}
+ /**
+ * Original cost formula: {@code COST_OVERHEAD + bestCount}. Ignores
+ * {@code costPerEntry}/{@code costPerExecution} even if set on the index
+ * definition.
+ */
+ double getCostLegacy(double bestCount) {
+ return bestCount == Double.POSITIVE_INFINITY ?
Double.POSITIVE_INFINITY : COST_OVERHEAD + bestCount;
Review Comment:
```suggestion
return bestCount == Long.MAX_VALUE ? Double.POSITIVE_INFINITY :
COST_OVERHEAD + bestCount;
```
And it looks like there needs to be a test case?
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java:
##########
@@ -205,6 +200,30 @@ String getName() {
return cost;
}
+ /**
+ * Original cost formula: {@code COST_OVERHEAD + bestCount}. Ignores
+ * {@code costPerEntry}/{@code costPerExecution} even if set on the index
+ * definition.
+ */
+ double getCostLegacy(double bestCount) {
Review Comment:
```suggestion
double getCostLegacy(long bestCount) {
```
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java:
##########
@@ -168,22 +163,22 @@ public class PropertyIndexPlan {
}
}
values = PropertyIndexUtil.encode(values);
- double cost = strategies.isEmpty() ? MAX_COST : 0;
+ long count = strategies.isEmpty() ? MAX_COUNT : 0;
Review Comment:
Now it's rather dangerous, because the switch from double to long also
impacts the old (legacy) code path (as you see later with comparison against
infinity / max value).
So it might be better to have a "legacy" method that is a copy of the old
code, and a second method that is the new code, and then one central switch
based on the feature toggle.
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java:
##########
@@ -205,6 +200,30 @@ String getName() {
return cost;
}
+ /**
+ * Original cost formula: {@code COST_OVERHEAD + bestCount}. Ignores
+ * {@code costPerEntry}/{@code costPerExecution} even if set on the index
+ * definition.
+ */
+ double getCostLegacy(double bestCount) {
+ return bestCount == Double.POSITIVE_INFINITY ?
Double.POSITIVE_INFINITY : COST_OVERHEAD + bestCount;
+ }
+
+ /**
+ * {@code cost = costPerExecution + costPerEntry * bestCount}, both
+ * optionally configured on the index definition (OAK-12348). Defaults
+ * ({@code costPerEntry=1.0}, {@code costPerExecution=COST_OVERHEAD})
+ * reproduce {@link #getCostLegacy} exactly.
+ */
+ double getCostConfigurable(double bestCount) {
Review Comment:
```suggestion
double getCostConfigurable(long bestCount) {
```
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java:
##########
@@ -205,6 +200,30 @@ String getName() {
return cost;
}
+ /**
+ * Original cost formula: {@code COST_OVERHEAD + bestCount}. Ignores
+ * {@code costPerEntry}/{@code costPerExecution} even if set on the index
+ * definition.
+ */
+ double getCostLegacy(double bestCount) {
+ return bestCount == Double.POSITIVE_INFINITY ?
Double.POSITIVE_INFINITY : COST_OVERHEAD + bestCount;
+ }
+
+ /**
+ * {@code cost = costPerExecution + costPerEntry * bestCount}, both
+ * optionally configured on the index definition (OAK-12348). Defaults
+ * ({@code costPerEntry=1.0}, {@code costPerExecution=COST_OVERHEAD})
+ * reproduce {@link #getCostLegacy} exactly.
+ */
+ double getCostConfigurable(double bestCount) {
+ if (bestCount == Double.POSITIVE_INFINITY) {
Review Comment:
MAX_VALUE
--
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]
