Aleksandr Efimov has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24699 )

Change subject: IMPALA-15266: Fix flaky test due to slow Calcite first query.
......................................................................


Patch Set 5:

(3 comments)

A couple of questions about the init path, nothing blocking. On the red check 
on PS5: the pipeline failed while fetching a child job's console (wget exit 8) 
- the jobs I can still see (ubuntu-20.04-build-only, the ARM build, 
auto-critic) are green, and the clang-tidy/rat/python-check builds have rotated 
away, so it may just need a re-trigger.

http://gerrit.cloudera.org:8080/#/c/24699/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java:

http://gerrit.cloudera.org:8080/#/c/24699/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java@55
PS5, Line 55:     ImpalaRelMetadataProvider.DEFAULT.revise(
Small thing about where this runs: Frontend's static init does 
CALCITE_COMPILER_FACTORY = loadCalciteCompilerFactory(), so this block executes 
on every impalad start, including clusters that never switch the Calcite 
planner on. Do you have a feel for what the Janino compilation adds there? If 
it is more than noise, doing it on a background thread, or only when the 
planner is enabled, would leave the startup path as it is for everyone else.


http://gerrit.cloudera.org:8080/#/c/24699/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteCompilerFactory.java@76
PS5, Line 76:     RelMetadataQuery.THREAD_PROVIDERS.set(
Do we still need this one? THREAD_PROVIDERS is a ThreadLocal, so it covers only 
the thread that loads the class, and each query thread gets its own value from 
RelOptCluster.create() and then from 
cluster_.setMetadataProvider(ImpalaRelMetadataProvider.DEFAULT) in the 
CalciteRelNodeConverter constructor - which is also what makes dropping the 
per-query set below safe. The revise() calls above are the part that warms 
Janino up, and this one uses DefaultRelMetadataProvider rather than the Impala 
provider, so it looks droppable to me.


http://gerrit.cloudera.org:8080/#/c/24699/5/tests/custom_cluster/test_automatic_invalidation.py
File tests/custom_cluster/test_automatic_invalidation.py:

http://gerrit.cloudera.org:8080/#/c/24699/5/tests/custom_cluster/test_automatic_invalidation.py@256
PS5, Line 256:     self.execute_query("select 1")
Out of curiosity: with the static init happening at startup now, do you know 
what is left initializing on the first query? You mention in the commit message 
that something still is, and if that is what keeps the first query near the 1s 
mark, the next test with a similar budget will need the same warm-up line - 
might be worth a JIRA if the profile shows an obvious remaining chunk.



--
To view, visit http://gerrit.cloudera.org:8080/24699
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I90b167055cb8d8b0045344085083160f1d98f5a4
Gerrit-Change-Number: 24699
Gerrit-PatchSet: 5
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aleksandr Efimov <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Sun, 23 Aug 2026 13:59:26 +0000
Gerrit-HasComments: Yes

Reply via email to