Skip to content

Disable stats and cost calculators by default#11765

Merged
arhimondr merged 2 commits intoprestodb:masterfrom
arhimondr:disable-stats-calculator
Oct 23, 2018
Merged

Disable stats and cost calculators by default#11765
arhimondr merged 2 commits intoprestodb:masterfrom
arhimondr:disable-stats-calculator

Conversation

@arhimondr
Copy link
Member

@arhimondr arhimondr commented Oct 23, 2018

Stats calculator fails on assertions for complex queries, thus it is not production ready yet.

The #11511 changes the place where the stats are computed to be
displayed in the final plan. Before this patch, the stats were computed in QueryMonitor, when printing a final plan.
Before if the stats couldn't be computed for whatever reason, only the text plan generation would fail.

Currently, when the stats calculator is invoked for every query during the initial planning, queries may
be failing, even when the CBO is not used.

This change disables stats calculator by default. It can be enabled back with a session property on per query basis.

@arhimondr
Copy link
Member Author

@arhimondr arhimondr force-pushed the disable-stats-calculator branch from c857db6 to 4863cd1 Compare October 23, 2018 16:15
Copy link
Contributor

@electrum electrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, but someone else should take a look

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems unrelated to making it disabled by default and should probably be a separate commit?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this basically a shortcut so you don't keep trying to calculate cost and coming up empty because all stats are unknown?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this basically a shortcut so you don't keep trying to calculate cost and coming up empty because all stats are unknown?

I'm really being paranoid here. Just want to make sure that this part of functionality is really killed for good.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extracted it into a separate commit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this basically a shortcut so you don't keep trying to calculate cost and coming up empty because all stats are unknown?

@arhimondr arhimondr force-pushed the disable-stats-calculator branch from 4863cd1 to 59a7410 Compare October 23, 2018 17:35
Stats calculator fails on assertions for complex queries, thus it is not production ready yet.

The prestodb#11511 changes the place where the stats are computed to be
displayed in the final plan. Before this patch, the stats were computed in QueryMonitor, when printing a final plan.
Before if the stats couldn't be computed for whatever reason, only the text plan generation would fail.

Currently, when the stats calculator is invoked for every query during the initial planning, queries may
be failing, even when the CBO is not used.

This change disables stats calculator by default. It can be enabled back with a session property on per query basis.
@arhimondr arhimondr force-pushed the disable-stats-calculator branch from 59a7410 to 4b46a29 Compare October 23, 2018 17:37
@arhimondr arhimondr merged commit e7b4b41 into prestodb:master Oct 23, 2018
@arhimondr arhimondr deleted the disable-stats-calculator branch October 23, 2018 18:53
@kokosing
Copy link
Contributor

As I understand this was found during the release verification and it was a release blocker, am I right?

@arhimondr
Copy link
Member Author

@kokosing Yeah =\ I'm actively working on fixing those bugs. Hopefully to the next release we will be able to turn it on back by default.

@findepi
Copy link
Contributor

findepi commented Oct 24, 2018

Stats calculator fails on assertions for complex queries

Would you be able to share a query or example stacktrace? In particular, is it an old problem?

(I am aware previously those bugs would be hidden as implicit EXPLAIN was async, but our release ships with CBO on by default, and I don't recall any planning failures due to that)

@arhimondr
Copy link
Member Author

arhimondr commented Oct 24, 2018 via email

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.

6 participants