[
https://issues.apache.org/jira/browse/DRILL-8555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18132943#comment-18132943
]
ASF GitHub Bot commented on DRILL-8555:
---------------------------------------
cgivre commented on code in PR #3086:
URL: https://github.com/apache/drill/pull/3086#discussion_r4389658093
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlWorker.java:
##########
@@ -295,7 +305,95 @@ private static PhysicalPlan getQueryPlan(QueryContext
context, String sql, Point
context.getOptions().setLocalOption(ExecConstants.RETURN_RESULT_SET_FOR_DDL,
true);
}
- return handler.getPlan(sqlNode);
+ // Keep the original SQL if no safe snapshot is available or cache
preparation fails.
+ SqlNode planningSql = sqlNode;
+ Consumer<PhysicalPlan> prepareCacheInsert = null;
+ if (context.getOptions().getOption(PlannerSettings.PLAN_CACHE)
+ && !context.getSession().hasTemporaryTables()
+ && !hasMutableAliases(context)
+ && PlanCacheEligibility.isSafeToCache(sqlNode,
context.getFunctionRegistry())) {
+ try {
+ PlanCache cache = context.getDrillbitContext().getPlanCache();
+ long cacheGeneration = cache.getGeneration();
+ // Capture the context once, before lookup and planning. Changes during
+ // planning invalidate this entry on its first hit.
+ PlanCache.ContextSnapshot snapshot = PlanCache.ContextSnapshot.resolve(
+ parser.getDefaultSchema(), sqlNode, context);
+ if (snapshot != null) {
+ PlanCacheParameterizer.Candidate candidate =
+ PlanCacheParameterizer.parameterize(sqlNode,
parser.getTypeFactory());
+ String key = context.getQueryUserName() + '\n'
+ + context.getSession().getDefaultSchemaPath() + '\n'
+ + snapshot.keyFingerprint() + '\n'
+ + candidate.template;
+ PlanCache.Entry entry = cache.get(key);
+ if (entry != null) {
+ try {
+ if (entry.matchesContext(snapshot)) {
+ PhysicalPlan bound = entry.bind(candidate.literals,
+ context.getDrillbitContext().getPlanReader());
+ PlanCache.bindTextPlan(textPlan, entry.getTextPlan(),
candidate.literals);
+ cache.recordBind();
+ context.setPlanCacheHit();
+ logger.debug("Plan cache hit");
+ return bound;
+ }
+ logger.debug("Plan cache entry invalidated because its context
changed; replanning");
+ } catch (RuntimeException | IOException e) {
+ logger.debug("Plan cache entry invalidated because rebinding
failed; replanning", e);
+ }
+ cache.invalidate(key);
+ } else {
+ logger.debug("Plan cache miss");
+ }
+ cache.recordMiss();
+ final PhysicalPlanReader cacheReader =
+ context.getDrillbitContext().getPlanReader();
+ // Prepare insertion after planning; Foreman publishes it only after
the query succeeds.
+ prepareCacheInsert = planned -> {
+ String templateTextPlan = PlanCache.bindTextPlan(textPlan,
candidate.literals);
+ final String json;
+ try {
+ // Capture before parallelization/execution mutates the
operators.
+ // Pending publication and the writer queue retain only
immutable JSON.
+ json = cacheReader.writeJson(planned);
+ } catch (IOException | RuntimeException e) {
+ logger.debug("Plan could not be serialized for the plan cache;
skipping publication", e);
+ return;
+ }
+ context.setPendingPlanCacheInsert(() ->
cache.writeAfterSuccess(key, json,
+ templateTextPlan, cacheReader, snapshot, cacheGeneration));
+ };
+ // On a cache miss, plan the parameterized SQL to produce a reusable
template.
+ planningSql = candidate.sql;
+ }
+ } catch (RuntimeException e) {
+ logger.debug("Plan cache attempt fell back to ordinary planning", e);
+ }
+ }
+
+ // Plan exactly once, outside cache fallback. Validation can mutate the
SQL tree,
+ // so retrying a failed planning attempt could reuse partially rewritten
nodes.
+ PhysicalPlan planned = handler.getPlan(planningSql);
Review Comment:
My biggest concern are queries randomly failing and not giving the user a
clear reason of what happened and how to fix it. But it sounds like that won't
happen here, so that's good!
> Physical plan cache for parameterized SQL queries
> -------------------------------------------------
>
> Key: DRILL-8555
> URL: https://issues.apache.org/jira/browse/DRILL-8555
> Project: Apache Drill
> Issue Type: New Feature
> Components: Query Planning & Optimization
> Reporter: Letian Jiang
> Priority: Major
>
> h2. Motivation
> Repeated queries that differ only in literals are a common workload, yet each
> query currently requires full planning. This adds unnecessary latency and
> wastes CPU time.
> h2. Brief Design
> Add a per-Drillbit, in-memory plan cache. Drill automatically parameterizes
> eligible SQL queries and reuses a cached physical plan with the current
> literal values, without changes to client queries.
> h2. Plan reuse safety
> * *Engine:* matches the SQL template and checks relevant option changes,
> plugin configuration changes, and whether base table versions still match.
> * *Storage plugin:* explicitly opts in, provides a plan-compatible table
> version, and is responsible for rebuilding its predicate-dependent scan work
> from the rebound expressions.
> h2. Expected benefit
> Short, frequently executed queries such as HBase reads can benefit
> substantially. Analytical queries with significant planning cost can benefit
> as well.
> h2. Implementation status
> A working implementation is available, including HBase, Paimon and Iceberg
> integrations. A pull request will follow.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
