Hi Bartos,
I checked the current evaluator code and you're correct that it exceeds the recursion depth for complex expressions.
Given your script, I was able to confirm that I'm hitting the same recursion depths as well.
Packrat[1] is an optimization (pyparsing.ParserElement.enablePackrat()) that uses memoization to reduce recursion depths but
that is already enabled in the evaluator (if not for it, simpler evaluations like "max(1, 2, 3)" wouldn't even return in 1000 recursion
depth and would fail). It has a cache size of 128 by default which I tried tweaking to make it unbounded but that didn't help.
Second approach which I tried was to use pyparsing_common instead of defining expressions in a custom way (as pyparsing already
has them optimized) which reduced the recursion depth by a measure of "10" with just redefining two variables.
I've proposed a patch for the same here[2] which is a minor change and thanks to your script, I was able to verify this improvement.
The main problem I see is with the infixNotation we use[3] as we're passing a lot of expressions to it which is known to reduce performance.
Thanks for raising the concern and feel free to open a bug at launchpad[4] and we can discuss this issue at our bug squad meeting.
Hello,
I wanted to share something I've recently discovered - there is a
strange issue with cinder evaluating complex configuration formulas.
For example when using complex goodness_function it's quite easy to hit:
ERROR cinder.scheduler.weights.goodness maximum recursion depth
exceeded: RecursionError: maximum recursion depth exceeded
I did some tests to find the root cause. It seems that pyparsing module
does deep recursive parsing quite a lot (default recursion limit is 1000).
Not sure how to solve it but it would be nice to at least have a warning
in the docs.
Results and methodology below.
Tests were done using following code snippet
-------------------------------------------
from cinder.scheduler.evaluator.evaluator import evaluate
import sys
formulas = [
"1",
"1 + 1",
"max(1, 2, 3)",
"max(1 + (10 / 20), 2, 3)",
"(1 + max(1 + (10 / 20), 2, 3)) / 100",
"((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb",
"(((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb)
/ 100",
]
stats = {
'allocated_capacity_gb': 50000,
'total_capacity_gb': 60000,
'free_capacity_gb': 10000,
}
for f in formulas:
for i in range(10, 2000, 10):
sys.setrecursionlimit(i)
try:
res = evaluate(f, stats=stats)
print(f"{i}: {f}")
break
except RecursionError:
pass
Test results
--------------------------------
Cinder 14.3.1, pyparsing<3.0
210: 1
210: 1 + 1
450: max(1, 2, 3)
600: max(1 + (10 / 20), 2, 3)
790: (1 + max(1 + (10 / 20), 2, 3)) / 100
980: ((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb
1160: (((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb)
/ 100
Cinder 16.4.2, pyparsing<3.0
220: 1
220: 1 + 1
450: max(1, 2, 3)
610: max(1 + (10 / 20), 2, 3)
790: (1 + max(1 + (10 / 20), 2, 3)) / 100
980: ((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb
1170: (((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb)
/ 100
Cinder 19.1.0, pyparsing<3.0
220: 1
220: 1 + 1
450: max(1, 2, 3)
610: max(1 + (10 / 20), 2, 3)
790: (1 + max(1 + (10 / 20), 2, 3)) / 100
980: ((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb
1170: (((1 + max(1 + (10 / 20), 2, 3)) / 100) + stats.total_capacity_gb)
/ 100
BR