Improve latency for HBO optimizer #21297

feilong-liu · 2023-11-02T04:56:43Z

Description

Part of #20355

This PR optimizes the latency for HBO optimizer from the following aspects:

Check timeout for plan canonicalization, plan hash and return empty when timeout
Previously it only checks timeout for meta data access, and do not return immediately when timeout. However, we found that plan canonicalization and hash can also be time consuming. And we need to check for these two, and also return immediately when timeout.
Add a flag to specify whether to fetch HBO related data from cache only. It's false when register the query, and true when getting statistics
In the design of HBO, all HBO related stats are fetched and cached during query registration in HistoricalStatisticsEquivalentPlanMarkingOptimizer optimizer, and following fetch of stats will access from cache. This PR adds a new parameter cacheOnly to corresponding methods to enforce this behavior.
Add verbose run time stats for plan hash, plan canonicalization and meta data read
This is only enabled when verbose runtime flag is set to true, and will help to debug HBO timeout.

Motivation and Context

This is to reduce the latency of HBO optimizer.

Impact

Tested with a few production queries, see significant drop of latency for HBO optimizer.

Test Plan

Add unit tests. Also tested locally end to end to verify that HBO optimizer latency decreased. In tests, queries which takes minutes to have HBO optimizer timeout now timeouts within specified timeout limit, i.e. 20seconds.

Contributor checklist

Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
Documented new properties (with its default value), SQL syntax, functions, or other functionality.
If release notes are required, they follow the release notes guidelines.
Adequate tests were added if applicable.
CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Improve latency of history based optimizer

feilong-liu · 2023-11-02T18:48:31Z

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java

+                if (enableVerboseRuntimeStats) {
+                    profileStartTime = System.nanoTime();
+                }
+                planNodesWithHash.addAll(getPlanNodeHashes(plan, session, false).values());


cache only is set to false, as this is during registration of plan, hence we are supposed to have nothing in cache and populate the cache for future use.

feilong-liu · 2023-11-02T18:48:59Z

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java

-            Optional<String> hash = planCanonicalInfoProvider.hash(session, statsEquivalentPlanNode, strategy);
-            allHashesBuilder.put(strategy, new PlanNodeWithHash(statsEquivalentPlanNode, hash));
+            Optional<String> hash = planCanonicalInfoProvider.hash(session, statsEquivalentPlanNode, strategy, cacheOnly);
+            if (hash.isPresent()) {


Only return hash when it's present, the returned hash can be empty if the "planCanonicalInfoProvider.hash" timeout.

feilong-liu · 2023-11-02T18:51:10Z

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java

@@ -147,7 +165,7 @@ private PlanNodeStatsEstimate getStatistics(PlanNode planNode, Session session,
        }

        PlanNode plan = resolveGroupReferences(planNode, lookup);
-        Map<PlanCanonicalizationStrategy, PlanNodeWithHash> allHashes = getPlanNodeHashes(plan, session);
+        Map<PlanCanonicalizationStrategy, PlanNodeWithHash> allHashes = getPlanNodeHashes(plan, session, true);


This is within "getStatistics" function which is called when reading estimates from HBO, and we set cacheOnly to true here, as we are supposed to already have cache here, and we do not want to calculate on the fly if cache miss during cost based planning.

feilong-liu · 2023-11-02T18:52:37Z

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java

@@ -163,7 +181,7 @@ private PlanNodeStatsEstimate getStatistics(PlanNode planNode, Session session,
        for (PlanCanonicalizationStrategy strategy : historyBasedPlanCanonicalizationStrategyList()) {
            for (Map.Entry<PlanNodeWithHash, HistoricalPlanStatistics> entry : statistics.entrySet()) {
                if (allHashes.containsKey(strategy) && entry.getKey().getHash().isPresent() && allHashes.get(strategy).equals(entry.getKey())) {
-                    Optional<List<PlanStatistics>> inputTableStatistics = getPlanNodeInputTableStatistics(plan, session);
+                    Optional<List<PlanStatistics>> inputTableStatistics = getPlanNodeInputTableStatistics(plan, session, true);


Similarly set to cacheOnly to true as it's within getStatistics function.

feilong-liu · 2023-11-02T18:59:18Z

presto-main/src/main/java/com/facebook/presto/sql/planner/CachingPlanCanonicalInfoProvider.java

        Map<CacheKey, PlanNodeCanonicalInfo> cache = historyBasedStatisticsCacheManager.getCanonicalInfoCache(session.getQueryId());
        PlanNodeCanonicalInfo result = cache.get(key);
-        if (result != null) {
-            return Optional.of(result);
+        if (result != null || cacheOnly) {


If cacheOnly set to true, return whatever got from cache. This field will be false during registration call in HBO optimizer, so that all data is calculated and cached during registration, and later usage will only read form cache.

feilong-liu · 2023-11-02T18:59:44Z

presto-main/src/main/java/com/facebook/presto/sql/planner/CachingPlanCanonicalInfoProvider.java

+            profileTime("CanonicalPlanGenerator", profileStartTime, session);
+        }
+        if (loadValueTimeout(startTimeInNano, timeoutInMilliseconds)) {
+            return Optional.empty();


Return early if timeout

feilong-liu · 2023-11-02T19:01:52Z

presto-main/src/main/java/com/facebook/presto/sql/planner/PlanNodeCanonicalInfo.java

-                Optional<String> hash = planCanonicalInfoProvider.hash(session, statsEquivalentPlanNode, strategy);
-                Optional<List<PlanStatistics>> inputTableStatistics = planCanonicalInfoProvider.getInputTableStatistics(session, statsEquivalentPlanNode);
+                Optional<String> hash = planCanonicalInfoProvider.hash(session, statsEquivalentPlanNode, strategy, true);
+                Optional<List<PlanStatistics>> inputTableStatistics = planCanonicalInfoProvider.getInputTableStatistics(session, statsEquivalentPlanNode, true);


This function is called at the end to collect the HBO information, hence cacheOnly set to true.

mlyublena

Nice finds!

tdcmeehan · 2023-11-03T15:23:52Z

Please improve the release note message (see guidelines). Suggestion:

Improve latency of history based optimizer

ajaygeorge

LGTM % a nit

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java

1. Check timeout for plan canonicalization, plan hash and return empty when timeout 2. Add a flag to specify whether to fetch HBO related data from cache only. It's false when register the query, and true when getting statistics 3. Add verbose run time stats for plan hash, plan canonicalization and meta data read

feilong-liu requested review from shrinidhijoshi and a team as code owners November 2, 2023 04:56

feilong-liu requested a review from presto-oss November 2, 2023 04:56

feilong-liu marked this pull request as draft November 2, 2023 04:56

feilong-liu force-pushed the debug_hbo_timeout_log branch from 947f845 to ed5640c Compare November 2, 2023 18:46

feilong-liu commented Nov 2, 2023

View reviewed changes

feilong-liu requested review from kaikalur, arhimondr, pranjalssh and mlyublena November 2, 2023 19:19

feilong-liu marked this pull request as ready for review November 2, 2023 19:20

mlyublena approved these changes Nov 2, 2023

View reviewed changes

feilong-liu requested a review from NikhilCollooru November 3, 2023 05:48

feilong-liu changed the title ~~Fix latency for HBO optimizer~~ Improve latency for HBO optimizer Nov 3, 2023

feilong-liu requested a review from ajaygeorge November 9, 2023 19:05

ajaygeorge approved these changes Nov 9, 2023

View reviewed changes

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java Outdated Show resolved Hide resolved

presto-main/src/main/java/com/facebook/presto/cost/HistoryBasedPlanStatisticsCalculator.java Outdated Show resolved Hide resolved

feilong-liu force-pushed the debug_hbo_timeout_log branch from ed5640c to 797b59e Compare November 9, 2023 23:53

feilong-liu force-pushed the debug_hbo_timeout_log branch from 797b59e to 6e30e08 Compare November 10, 2023 00:24

shrinidhijoshi approved these changes Nov 10, 2023

View reviewed changes

feilong-liu merged commit 8815076 into prestodb:master Nov 10, 2023

feilong-liu deleted the debug_hbo_timeout_log branch November 10, 2023 06:10

wanglinsong mentioned this pull request Dec 8, 2023

Add release notes for 0.285 #21500

Closed

26 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve latency for HBO optimizer #21297

Improve latency for HBO optimizer #21297

feilong-liu commented Nov 2, 2023 •

edited

Loading

feilong-liu Nov 2, 2023

feilong-liu Nov 2, 2023

feilong-liu Nov 2, 2023

feilong-liu Nov 2, 2023

feilong-liu Nov 2, 2023

feilong-liu Nov 2, 2023

feilong-liu Nov 2, 2023

mlyublena left a comment

tdcmeehan commented Nov 3, 2023 •

edited

Loading

ajaygeorge left a comment

Improve latency for HBO optimizer #21297

Improve latency for HBO optimizer #21297

Conversation

feilong-liu commented Nov 2, 2023 • edited Loading

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

Release Notes

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

feilong-liu Nov 2, 2023

Choose a reason for hiding this comment

mlyublena left a comment

Choose a reason for hiding this comment

tdcmeehan commented Nov 3, 2023 • edited Loading

ajaygeorge left a comment

Choose a reason for hiding this comment

feilong-liu commented Nov 2, 2023 •

edited

Loading

tdcmeehan commented Nov 3, 2023 •

edited

Loading