Steve Carlin 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) 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_COMP I think it's fine. When the project first started, we had this initial thought; that Calcite would not necessarily be on every server. This is one of the reasons it is in its own jar file. At some point, we made the decision to include it. But my thought here is that if we are loading the Calcite jar file, then it's fair game to initialize all variables as well. Indeed, we are already initializing the ones in the Impala Calcite jar file. The Janino compilation, as far as I can tell, is generating code for the metadata queries. This is code that theoretically should be there anyway. My guess as to why it is compiled code instead of actual code is that they were worried about code bloat? In earlier versions, I think they generated code for each RelNode for each MetadataQuery, and maybe some systems had a ton of different RelNodes? Just a guess here. ButI don't think it's a huge hit memory-wise with our limited MetadataQueries (and even our limited RelNodes, but I don't think that matters anymore). So I'm not too worried about this being done for all servers, even if they don't use Calcite. 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 Removed the code 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 kno Filed IMPALA-15283 as a catch-all to find these. -- 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-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Sun, 23 Aug 2026 15:11:10 +0000 Gerrit-HasComments: Yes
