Skip to content

[CALCITE-3257] RelMetadataQuery cache is not invalidated when log trace is enabled#1385

Closed
xndai wants to merge 3 commits intoapache:masterfrom
xndai:clean_meta_cache
Closed

[CALCITE-3257] RelMetadataQuery cache is not invalidated when log trace is enabled#1385
xndai wants to merge 3 commits intoapache:masterfrom
xndai:clean_meta_cache

Conversation

@xndai
Copy link
Contributor

@xndai xndai commented Aug 16, 2019

Based on current design, RelMetadataQuery.map needs to be cleared between each rule firing. This is achieved through RelOptCluster.invalidateMetadataQuery() by VolcanoRuleCall.transformTo(). But when trace is enabled, the dump process would actually rebuild the meta data cache from previous rel tree. Then the subsequent rule firing doesn't get a chance to update rel node cost as it's been in the cache.

A simple fix will just add a call to RelOptCluster.invalidateMetadataQuery() after dumping rel nodes.

https://issues.apache.org/jira/browse/CALCITE-3257

…Nodes and Graphviz

Based on current design, RelMetadataQuery.map needs to be cleared between each rule firing. This is achieved through RelOptCluster.invalidateMetadataQuery() by VolcanoRuleCall.transformTo(). But when trace is enabled, the dump process would actually rebuild the meta data cache from previous rel tree. Then the subsequent rule firing doesn't get a chance to update rel node cost as it's been in the cache.
A simple fix will just add a call to RelOptCluster.invalidateMetadataQuery() after dumping rel nodes.
@julianhyde
Copy link
Contributor

Please change the description "Need to..." is not accurate. This is a workaround for another issue. Please add the necessary context.

xndai added 2 commits August 19, 2019 11:16
…ce is enabled

Based on current design, RelMetadataQuery.map needs to be cleared between each rule firing. This is achieved through RelOptCluster.invalidateMetadataQuery() by VolcanoRuleCall.transformTo(). But when trace is enabled, the dump process would actually rebuild the meta data cache from previous rel tree. Then the subsequent rule firing doesn't get a chance to update rel node cost as it's been in the cache.
A simple fix will just add a call to RelOptCluster.invalidateMetadataQuery() after dumping rel nodes.
@xndai xndai changed the title [CALCITE-3257] Need to clear RelMetaDataQuery cache after dumping Rel… [CALCITE-3257] RelMetadataQuery cache is not invalidated when log trace is enabled Aug 19, 2019
@xndai xndai closed this Aug 19, 2019
@xndai xndai reopened this Aug 19, 2019
@xndai
Copy link
Contributor Author

xndai commented Aug 20, 2019

hi @julianhyde , this PR is to ensure we purge meta data cache when trace is enabled, same as when trace is disabled. CALCITE-2018 is to fix issue during propogateCostImprovement(). @vvysotskyi also agrees they are two different issues. More details please refer to the Jira. Thanks.

Copy link
Member

@hsyuan hsyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@hsyuan hsyuan added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Aug 21, 2019
@hsyuan hsyuan closed this in ab97af3 Aug 22, 2019
wangxlong pushed a commit to wangxlong/calcite that referenced this pull request Feb 13, 2020
…ce is enabled (Xiening Dai)

Based on current design, RelMetadataQuery.map needs to be cleared between each
rule firing. This is achieved through RelOptCluster.invalidateMetadataQuery()
by VolcanoRuleCall.transformTo(). But when trace is enabled, the dump process
would actually rebuild the meta data cache from previous rel tree. Then the
subsequent rule firing doesn't get a chance to update rel node cost as it's
been in the cache.

A simple fix will just add a call to RelOptCluster.invalidateMetadataQuery()
after dumping rel nodes.

Close apache#1385
hbtoo pushed a commit to hbtoo/calcite that referenced this pull request Oct 2, 2020
…ce is enabled (Xiening Dai)

Based on current design, RelMetadataQuery.map needs to be cleared between each
rule firing. This is achieved through RelOptCluster.invalidateMetadataQuery()
by VolcanoRuleCall.transformTo(). But when trace is enabled, the dump process
would actually rebuild the meta data cache from previous rel tree. Then the
subsequent rule firing doesn't get a chance to update rel node cost as it's
been in the cache.

A simple fix will just add a call to RelOptCluster.invalidateMetadataQuery()
after dumping rel nodes.

Close apache#1385

Change-Id: Ibc09f391632c1e6fdfd5ce40f1a1762584033357
jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Aug 28, 2025
…ce is enabled (Xiening Dai)

Based on current design, RelMetadataQuery.map needs to be cleared between each
rule firing. This is achieved through RelOptCluster.invalidateMetadataQuery()
by VolcanoRuleCall.transformTo(). But when trace is enabled, the dump process
would actually rebuild the meta data cache from previous rel tree. Then the
subsequent rule firing doesn't get a chance to update rel node cost as it's
been in the cache.

A simple fix will just add a call to RelOptCluster.invalidateMetadataQuery()
after dumping rel nodes.

Close apache#1385

Change-Id: I2dd3965e54d2927732ef7ecd8ab2264a98a5626f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants