Skip to content

Commit 3234243

Browse files
kaijiandingmergify[bot]
authored andcommitted
[BugFix] fix mem leak when caching mv plan context (#72300)
Signed-off-by: kaijian.ding <kaijian.ding@gmail.com> (cherry picked from commit 33ccc4a) # Conflicts: # fe/fe-core/src/test/java/com/starrocks/planner/MaterializedViewTextBasedRewriteTest.java
1 parent 8c446e5 commit 3234243

2 files changed

Lines changed: 162 additions & 1 deletion

File tree

fe/fe-core/src/main/java/com/starrocks/sql/optimizer/CachingMvPlanContextBuilder.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,11 @@ public boolean contains(MaterializedView mv) {
304304
return MV_PLAN_CONTEXT_CACHE.asMap().containsKey(mv);
305305
}
306306

307+
@VisibleForTesting
308+
public boolean isPending(MaterializedView mv) {
309+
return MV_PLAN_CACHE_PENDING.containsKey(mv.getId());
310+
}
311+
307312
/**
308313
* Cache materialized view, this will put the mv into ast cache and load plan context asynchronously.
309314
* @param mv: the materialized view to cache.
@@ -328,6 +333,9 @@ public void evictMaterializedViewCache(MaterializedView mv) {
328333
// invalidate mv from plan cache
329334
MV_PLAN_CONTEXT_CACHE.synchronous().invalidate(mv);
330335

336+
// invalidate mv from pending cache
337+
MV_PLAN_CACHE_PENDING.remove(mv.getId());
338+
331339
// invalidate mv from mv level cache
332340
MV_GLOBAL_CONTEXT_CACHE_MAP.remove(mv);
333341

@@ -400,7 +408,15 @@ public void triggerPendingMVPlanCacheLoads() {
400408
}
401409
long startTime = System.currentTimeMillis();
402410
LOG.info("Trigger loading {} pending mv plan caches", MV_PLAN_CACHE_PENDING.size());
403-
MV_PLAN_CACHE_PENDING.values().forEach(this::triggerLoadMVPlanCacheAsync);
411+
MV_PLAN_CACHE_PENDING.values().forEach(mv -> {
412+
// Re-fetch from metastore to skip MVs that were dropped during replay.
413+
MaterializedView curMV = GlobalStateMgr.getCurrentState().getLocalMetastore().getMaterializedView(mv.getMvId());
414+
if (curMV == null) {
415+
LOG.warn("Skip pending mv plan cache load, mv not found in metastore: {}", mv.getName());
416+
return;
417+
}
418+
triggerLoadMVPlanCacheAsync(curMV);
419+
});
404420
MV_PLAN_CACHE_PENDING.clear();
405421
LOG.info("Finish triggering pending mv plan caches, costs: {}ms",
406422
System.currentTimeMillis() - startTime);

fe/fe-core/src/test/java/com/starrocks/planner/MaterializedViewTextBasedRewriteTest.java

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,25 @@
1616

1717
import com.starrocks.analysis.ParseNode;
1818
import com.starrocks.catalog.MaterializedView;
19+
<<<<<<< HEAD
20+
=======
21+
import com.starrocks.common.jmockit.Deencapsulation;
22+
import com.starrocks.server.GlobalStateMgr;
23+
import com.starrocks.sql.ast.ParseNode;
24+
>>>>>>> 33ccc4ae03 ([BugFix] fix mem leak when caching mv plan context (#72300))
1925
import com.starrocks.sql.common.QueryDebugOptions;
2026
import com.starrocks.sql.optimizer.CachingMvPlanContextBuilder;
2127
import com.starrocks.sql.optimizer.rule.transformation.materialization.MvUtils;
2228
import com.starrocks.sql.plan.PlanTestBase;
29+
import mockit.Mock;
30+
import mockit.MockUp;
2331
import org.junit.jupiter.api.Assertions;
2432
import org.junit.jupiter.api.BeforeAll;
2533
import org.junit.jupiter.api.MethodOrderer;
2634
import org.junit.jupiter.api.Test;
2735
import org.junit.jupiter.api.TestMethodOrder;
2836

37+
import java.util.Map;
2938
import java.util.Set;
3039

3140
@TestMethodOrder(MethodOrderer.MethodName.class)
@@ -429,4 +438,140 @@ public void testGetMvsByAstReturnsEmptySetWhenNotInMap() throws Exception {
429438
Assertions.assertEquals(expectedMV, actualMV);
430439
}
431440
}
441+
442+
/**
443+
* Regression test for the bug where a dropped MV still remains in MV_PLAN_CACHE_PENDING.
444+
*
445+
* Scenario (mirrors replay order during FE startup when isReady=false):
446+
* 1. MV is created → mv set active → cacheMaterializedView() → triggerLoadMVPlanCacheAsync() → enters pending
447+
* 2. MV is dropped → evictMaterializedViewCache() must also remove it from pending
448+
*
449+
* Without the fix, the dropped MV remains in MV_PLAN_CACHE_PENDING.
450+
*/
451+
@Test
452+
public void testEvictRemovesMvFromPendingCache() throws Exception {
453+
new MockUp<GlobalStateMgr>() {
454+
@Mock
455+
public boolean isReady() {
456+
return false;
457+
}
458+
};
459+
String mvName = "test_mv_evict_pending";
460+
String createMvSql = "create materialized view " + mvName + " distributed by random refresh manual " +
461+
"as select user_id, time, sum(tag_id) from user_tags group by user_id, time";
462+
463+
starRocksAssert.withMaterializedView(createMvSql);
464+
MaterializedView mv = getMv(MATERIALIZED_DB_NAME, mvName);
465+
try {
466+
mv.setActive();
467+
468+
Assertions.assertTrue(CachingMvPlanContextBuilder.getInstance().isPending(mv),
469+
"MV should be in pending after triggerLoadMVPlanCacheAsync with isReady=false");
470+
} finally {
471+
starRocksAssert.dropMaterializedView(mvName);
472+
473+
Assertions.assertFalse(CachingMvPlanContextBuilder.getInstance().isPending(mv),
474+
"MV should be removed from pending after evictMaterializedViewCache");
475+
}
476+
}
477+
478+
/**
479+
* Regression test for the bug where a dropped MV still remains in MV_PLAN_CACHE_PENDING.
480+
*
481+
* Scenario (mirrors replay order during FE startup when isReady=false):
482+
* 1. MV is created → mv set active → cacheMaterializedView() → triggerLoadMVPlanCacheAsync() → enters pending
483+
* 2. MV is dropped → evictMaterializedViewCache() must also remove it from pending
484+
* 3. After FE ready → triggerPendingMVPlanCacheLoads() must not re-insert the dropped MV
485+
*
486+
* Without the fix, step 3 would load the stale MV object into MV_PLAN_CONTEXT_CACHE.
487+
*/
488+
@Test
489+
public void testEvictRemovesMvFromPendingCache2() throws Exception {
490+
new MockUp<GlobalStateMgr>() {
491+
@Mock
492+
public boolean isReady() {
493+
return false;
494+
}
495+
};
496+
String mvName = "test_mv_evict_pending";
497+
String createMvSql = "create materialized view " + mvName + " distributed by random refresh manual " +
498+
"as select user_id, time, sum(tag_id) from user_tags group by user_id, time";
499+
500+
starRocksAssert.withMaterializedView(createMvSql);
501+
MaterializedView mv = getMv(MATERIALIZED_DB_NAME, mvName);
502+
try {
503+
mv.setActive();
504+
505+
// MV should now be in pending.
506+
Assertions.assertTrue(CachingMvPlanContextBuilder.getInstance().isPending(mv),
507+
"MV should be in pending after triggerLoadMVPlanCacheAsync with isReady=false");
508+
} finally {
509+
new MockUp<GlobalStateMgr>() {
510+
@Mock
511+
public boolean isReady() {
512+
return true;
513+
}
514+
};
515+
starRocksAssert.dropMaterializedView(mvName);
516+
517+
// Simulate the racing condition: MV was dropped during replay, but its stale entry is still
518+
// present in MV_PLAN_CACHE_PENDING
519+
Map<Long, MaterializedView> pendingMap =
520+
Deencapsulation.getField(CachingMvPlanContextBuilder.getInstance(), "MV_PLAN_CACHE_PENDING");
521+
pendingMap.put(mv.getId(), mv);
522+
523+
CachingMvPlanContextBuilder.getInstance().triggerPendingMVPlanCacheLoads();
524+
525+
Assertions.assertFalse(CachingMvPlanContextBuilder.getInstance().isPending(mv),
526+
"MV should be removed from pending after triggerPendingMVPlanCacheLoads");
527+
528+
Assertions.assertFalse(CachingMvPlanContextBuilder.getInstance().contains(mv),
529+
"MV should not be cached after triggerPendingMVPlanCacheLoads");
530+
}
531+
}
532+
533+
/**
534+
* Normal path
535+
*
536+
* Scenario (mirrors replay order during FE startup when isReady=false):
537+
* 1. MV is created → mv set active → cacheMaterializedView() → triggerLoadMVPlanCacheAsync() → enters pending
538+
* 2. After FE ready → triggerPendingMVPlanCacheLoads() must re-insert the MV
539+
*
540+
*/
541+
@Test
542+
public void testEvictRemovesMvFromPendingCache3() throws Exception {
543+
new MockUp<GlobalStateMgr>() {
544+
@Mock
545+
public boolean isReady() {
546+
return false;
547+
}
548+
};
549+
String mvName = "test_mv_evict_pending";
550+
String createMvSql = "create materialized view " + mvName + " distributed by random refresh manual " +
551+
"as select user_id, time, sum(tag_id) from user_tags group by user_id, time";
552+
553+
starRocksAssert.withMaterializedView(createMvSql, name -> {
554+
MaterializedView mv = getMv(MATERIALIZED_DB_NAME, mvName);
555+
mv.setActive();
556+
557+
// MV should now be in pending.
558+
Assertions.assertTrue(CachingMvPlanContextBuilder.getInstance().isPending(mv),
559+
"MV should be in pending after triggerLoadMVPlanCacheAsync with isReady=false");
560+
561+
new MockUp<GlobalStateMgr>() {
562+
@Mock
563+
public boolean isReady() {
564+
return true;
565+
}
566+
};
567+
568+
CachingMvPlanContextBuilder.getInstance().triggerPendingMVPlanCacheLoads();
569+
570+
Assertions.assertFalse(CachingMvPlanContextBuilder.getInstance().isPending(mv),
571+
"MV should be removed from pending after triggerPendingMVPlanCacheLoads");
572+
573+
Assertions.assertTrue(CachingMvPlanContextBuilder.getInstance().contains(mv),
574+
"MV should be cached after triggerPendingMVPlanCacheLoads");
575+
});
576+
}
432577
}

0 commit comments

Comments
 (0)