github-actions[bot] commented on code in PR #68141:
URL: https://github.com/apache/doris/pull/68141#discussion_r4071428843
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -523,51 +527,56 @@ public Set<TableNameInfo>
getQueryRewriteConsistencyRelaxedTables() {
*/
public MTMVCache getOrGenerateCache(ConnectContext connectionContext)
throws
org.apache.doris.nereids.exceptions.AnalysisException {
- // store two MTMVCaches: one is a cache where SessionVariables differ
from those at creation time,
- // and the MTMV plan includes a guardexpr;
- // the other is a cache where SessionVariables are the same as at
creation time, and the MTMV plan
- // does not include a guardexpr;
- // This way, when sessionVariables are the same, rewriting is possible;
- // When sessionVariables are different, there are two cases:
- // 1. If a guardexpr is present, rewriting is not possible;
- // 2. If no guardexpr is present, rewriting is possible.
- // Determine if current session variables match MV creation session
variables
Map<String, String> currentSessionVars =
connectionContext.getSessionVariable().getAffectQueryResultInPlanVariables();
boolean sessionVarsMatch =
SessionVarGuardRewriter.checkSessionVariablesMatch(
currentSessionVars, this.sessionVariables);
+ boolean guarded = !sessionVarsMatch;
+ MTMVCacheManager manager = Env.getCurrentEnv().getMtmvCacheManager();
+ StatementContext statementContext =
connectionContext.getStatementContext();
while (true) {
long cacheGeneration;
- // Select appropriate cache based on session variable match
+ MTMVCache cached;
readMvLock();
try {
- MTMVCache cache = getCache(sessionVarsMatch);
- if (cache != null) {
- return cache;
+ cached = manager.isEnabled() ? manager.getIfPresent(this.id,
guarded) : null;
+ if (cached == null && statementContext != null) {
+ cached = statementContext.getQueryLocalMtmvCache(this.id,
guarded);
Review Comment:
[P1] Validate statement-local hits against the rewrite generation. With
`mtmv_cache_manage_num=0`, one planning attempt can store a generation-G plan
here, then a concurrent refresh or ADD/DROP CONSTRAINT advances the MTMV to G+1
and invalidates only the Env-wide manager. A later lookup using the same
`StatementContext` returns this G entry before comparing any generation:
```text
Query / replan (same StatementContext)
LogicalOlapScan(MV) -- computeUnique stores cache G
concurrent DROP CONSTRAINT -- rewriteCacheGeneration becomes G+1
same scan -- computeFd/constructReplaceMap returns cache G
```
That can preserve obsolete uniqueness/FK-derived traits or old rewrite
`StructInfo` after the transition intended to invalidate them. This is distinct
from the earlier global-hit race because `manager.invalidate()` cannot reach
`queryLocalMtmvCaches`. Store the generation alongside each local value and
accept it only when it matches the generation read under `readMvLock()`; add a
max-zero test that fills the local entry, invalidates or refreshes, and proves
the next same-context lookup rebuilds.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVCacheManager.java:
##########
@@ -0,0 +1,210 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.mtmv;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.common.Config;
+import org.apache.doris.common.ConfigBase.DefaultConfHandler;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.stats.CacheStats;
+import com.google.common.annotations.VisibleForTesting;
+
+import java.lang.reflect.Field;
+import java.time.Duration;
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
+
+/**
+ * FE-local cache manager for materialized view cache.
+ */
+public class MTMVCacheManager {
+
+ private final Object swapLock = new Object();
+ private volatile Cache<Key, MTMVCache> caches;
+
+ public MTMVCacheManager() {
+ caches = build(Config.mtmv_cache_manage_num,
Config.expire_mtmv_cache_in_fe_second);
+ }
+
+ public MTMVCache getIfPresent(long mtmvId, boolean guarded) {
+ return caches.getIfPresent(new Key(mtmvId, guarded));
+ }
+
+ public void put(long mtmvId, boolean guarded, MTMVCache cache) {
+ Objects.requireNonNull(cache, "mtmv cache to publish must not be
null");
+ synchronized (swapLock) {
+ caches.put(new Key(mtmvId, guarded), cache);
+ }
+ }
+
+ public void invalidate(long mtmvId) {
+ synchronized (swapLock) {
+ caches.invalidate(new Key(mtmvId, true));
+ caches.invalidate(new Key(mtmvId, false));
+ }
+ }
+
+ public void invalidateAll() {
+ synchronized (swapLock) {
+ caches.invalidateAll();
+ }
+ }
+
+ public long size() {
+ return caches.estimatedSize();
+ }
+
+ /** False when the live maximum is 0, i.e. every put would be discarded
immediately. */
+ public boolean isEnabled() {
+ return caches.policy().eviction().map(eviction ->
eviction.getMaximum() > 0).orElse(true);
+ }
+
+ public Snapshot snapshot() {
+ Cache<Key, MTMVCache> current = caches;
+ CacheStats s = current.stats();
+ return new Snapshot(current.estimatedSize(), s.hitCount(),
s.missCount(),
Review Comment:
[P2] Run pending maintenance before reporting the live cache size. [Caffeine
3.2.3's `estimatedSize()`
contract](https://github.com/ben-manes/caffeine/blob/v3.2.3/caffeine/src/main/java/com/github/benmanes/caffeine/cache/Cache.java#L167-L170)
allows expired and soft-collected entries pending removal to remain in the
estimate; this cache uses both `expireAfterAccess` and `softValues()`. For an
otherwise idle cache, the only entry can expire and stop being returned while
`SHOW PROC '/mtmv_cache/stat'` still reports `size=1`, making the new
diagnostic misleading precisely when an operator checks whether expiration
worked. Capture `current` as you do now, call `current.cleanUp()` before taking
its stats and size (or explicitly label the value approximate), and add a
fake-ticker expiry test for the snapshot.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
