[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9758: IGNITE-16354 Fix memory leak in RelOptCluster during mapping

[GitBox <gi...@apache.org>]

alex-plekhanov commented on a change in pull request #9758:
URL: https://github.com/apache/ignite/pull/9758#discussion_r791806082



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/MappingQueryContext.java
##########
@@ -45,4 +53,16 @@ public UUID localNodeId() {
     public AffinityTopologyVersion topologyVersion() {
         return topVer;
     }
+
+    /** Creates a cluster. */
+    RelOptCluster cluster() {
+        if (cluster == null) {
+            cluster = RelOptCluster.create(Commons.emptyCluster().getPlanner(), Commons.emptyCluster().getRexBuilder());

Review comment:
       The planner shouldn't be used by the empty cluster at all, there are assertions to prevent this (I've looked at the fix to ignite-3, and looks like the same check was implemented but in a different way, by prohibiting registerSchema invocation). I can add the same check to the planner as well, but now from my point of view, it looks redundant.
   The current fix prohibits not only planning but also mapping (mapping use metadataQuerySupplier, I'm not sure mapping uses planner somehow). 
   Also, I think the fix for ignite-3 is not complete, we should attach fragments to the empty cluster again before storing it   to `executionPlan` variable (see https://github.com/apache/ignite/pull/9758/files#diff-07fea6ecf0f238df45e8d6fdffabf83cf9a5fdbd90b31f897868a282c0694f0eR99), otherwise cluster with cached metadata can't be garbage collected (referenced from query cache -> query template -> execution plan -> fragments -> cluster)  




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org