Skip history optimizer query registration if previous registration timeout #21308
Conversation
This file contains 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
feilong-liu
force-pushed
the
no_double_hbo_timeout
branch
from
86be733
to
3269e1e
Compare
feilong-liu
commented
Nov 3, 2023
| @@ -142,7 +151,7 @@ private Map<PlanCanonicalizationStrategy, PlanNodeWithHash> getPlanNodeHashes(Pl | |||
|
|
|||
| private PlanNodeStatsEstimate getStatistics(PlanNode planNode, Session session, Lookup lookup, PlanNodeStatsEstimate delegateStats) | |||
| { | |||
| if (!useHistoryBasedPlanStatisticsEnabled(session) || historyBasedStatisticsCacheManager.loadHistoryFailed(session.getQueryId())) { | |||
There was a problem hiding this comment.
loadHistoryFailed is a macro optimization for HBO latency added in #19933. For queries which do not load any history statistics, we will skip attempt to read history stats. However, by the design, all statistics are cached during query registration, and the read here will be reading from cache, hence the saving is small. On the other hand, we run HBO optimizer twice, having one run reading empty does not mean the second time will still be empty. Hence remove this macro optimization here.
feilong-liu
requested review from
kaikalur,
pranjalssh,
jainxrohit and
mlyublena
jaystarshot
approved these changes
Nov 6, 2023
mlyublena
approved these changes
Nov 6, 2023
This cache adds a query ID when HBO failed to fetch history stats for this query. Later during cost based planning, it will skip reading history stats. However, the saving is small as the reading will be from cache. And currently we run HBO optimizer twice, getting empty stats for one run does not necessarily mean getting empty stats for the other run. This is an over optimization and I remove it here.
Currently we run HBO optimizer twice. When the first run timeout, it's likely that the second run will timeout too. Here we record the timeout, and skip registration for the next run.
feilong-liu
force-pushed
the
no_double_hbo_timeout
branch
from
3269e1e
to
16eee6d
Compare
26 tasks
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.
Description
Part of #20355
In current code, HBO optimizer runs twice. If in the first run, the query registration timeout, it's very likely that the second run will timeout too. Instead of run both and timeout twice, in this PR, I add code to record the timeout of the first run, so that the second run will skip if the first run timeout.
Motivation and Context
Reduce latency for HBO.
Impact
HBO latency for queries which timeout during HBO query registration will be half as of now. For example, if timeout limit is 10 seconds, and the total latency caused by HBO optimizer which timeout will be 2*10 = 20 seconds. After this change, it will be 10 seconds.
Test Plan
Test locally end to end, optimizer latency decreased from 25.44s to 10.2s
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.