[BugFix] fix mem leak when caching mv plan context#72300
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>
a8bf6c2 to
4f0f259
Compare
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Fixes a FE-side memory leak where dropped materialized views could remain referenced via the MV plan-context pending cache during replay/startup, causing stale/heavy MV objects to be retained and potentially loaded into the context cache later.
Changes:
- Remove evicted/dropped MVs from
MV_PLAN_CACHE_PENDINGduringevictMaterializedViewCache(). - When triggering pending loads, re-fetch MV metadata from
LocalMetastoreand skip loading if the MV no longer exists. - Add regression tests covering pending-cache eviction and replay “pending load after drop” scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| fe/fe-core/src/main/java/com/starrocks/sql/optimizer/CachingMvPlanContextBuilder.java | Ensures dropped MVs don’t linger in pending cache; avoids loading stale pending entries by re-validating against metastore. |
| fe/fe-core/src/test/java/com/starrocks/planner/MaterializedViewTextBasedRewriteTest.java | Adds regression tests to prevent reintroducing pending-cache retention and stale-load issues. |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
4f0f259 to
112bdfc
Compare
Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>
112bdfc to
17f17bf
Compare
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 9 / 9 (100.00%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
|
@Mergifyio backport branch-4.1 |
|
@Mergifyio backport branch-3.5 |
|
@Mergifyio backport branch-4.0 |
✅ Backports have been createdDetails
|
✅ Backports have been createdDetails
Cherry-pick of 33ccc4a has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
✅ Backports have been createdDetails
Cherry-pick of 33ccc4a has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Signed-off-by: kaijian.ding <kaijian.ding@gmail.com>
Signed-off-by: kaijian.ding <kaijian.ding@gmail.com> Signed-off-by: xiangguangyxg <xiangguangyxg@gmail.com>
…72390) Co-authored-by: kaijianding <kaijian.ding@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kevincai <771299+kevincai@users.noreply.github.com>
Why I'm doing:
There are lots of
LakeTabletobject on FOLLOWER after frequently drop&create same materialized view.And objects can't be releazed after fe transfers to LEADER.
What I'm doing:
Removed dropped materialized view object from pending cache and double check materialized view still exists before be added to context cache.
Fixes #issue
What type of PR is this:
Does this PR entail a change in behavior?
Checklist:
Bugfix cherry-pick branch check: