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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
