[TRAFODION-2661] Improve MDAM costing for RangeSpecs #1146
Conversation
|
Please do not merge this change yet. I am running full regressions on a workstation overnight with this change. |
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/1889/ |
There was a problem hiding this comment.
+1 I am not an expert on costing, but the explanation sounds good to me. I do wonder whether we have a problem in costing a RangeSpec, which may include multiple positions, unlike other predicates that typically appear in a column for a disjunct. Even if so, this seems the right way forward.
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/1889/ |
|
@zellerh, you raise a good point. I should do a bit more due diligence on this. Please do not merge yet. |
|
Full regressions passed overnight. But please do not merge yet; I want to do some due diligence on @zellerh's remarks. |
|
I will be submitting additional changes for this pull request, so please do not merge. |
This rework fixes a bug in the pre-code-gen time code. The bug was that in the presence of partitioning key predicates, logic that communicates stop column information to the generator was being bypassed. This would result in traversal to the last key predicate at run time, with poor performance if the stop column was prior to the last key predicate. Also included in this rework are some tweaks to the run-time debug code for MDAM (the code had become non-functional from not being used for a long time), and a fix to a run-time memory leak bug that shows up as an assert when that debug code is present.
|
Notes about the rework for [TRAFODION-2661]: |
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/1915/ |
|
I will be running full regressions on this change overnight on a workstation, so please do not merge until I give the "all clear" on that. |
|
+1 |
|
Test Failed. https://jenkins.esgyn.com/job/Check-PR-master/1915/ |
|
The test failure was in seabase/TEST010, which I believe has been separately addressed by #1161. |
|
I have run this through the full regression suite on a workstation instance and it passes (apart from a couple of false negatives). This can be merged now. |
JIRA TRAFODION-1641 fixed an issue with MDAM costing, where MDAM costing was not accounting for the cost of recursion. That is, it was not accounting for the accumulated costs of probes as we recurse through a set of key columns. When analyzing that JIRA, I uncovered an issue with RangeSpec costing which I partially fixed.
JIRA TRAFODION-2655 tuned the above fix further, increasing the weight of the cost of accumulated probes. So for plans lacking RangeSpecs, or plans whose RangeSpecs lack ORs, MDAM costing should now be doing a much better job of picking prefixes when it should.
This fix addresses the last bit of the problem by addressing RangeSpecs. There was some code that was forcing MDAM plans to traverse all key columns when a RangeSpec containing an OR was present. I have simply deleted that code.
I do not know why that code was originally there. It does predate the fixes noted above. I will speculate that before that change, we were picking MDAM in some cases where we should not, and adding that bit of code increased the total cost of MDAM to such a point that we no longer did.