This is an automated email from the ASF dual-hosted git repository. yuqi1129 pushed a commit to branch fix/12377-expire-after-write in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 458a3122dd68e424f58cf24844beda9594b2dace Author: yuqi <[email protected]> AuthorDate: Fri Sep 11 17:56:20 2026 +0800 [#12377] fix(core): expire entity cache entries after write, not after access CaffeineEntityCache built its cache with expireAfterAccess while the config `gravitino.cache.expireTimeInMs` and the docs both describe a TTL measured from the write. With an access-based TTL, a stale entry that keeps being read never expires, so a single missed cross-node invalidation becomes permanent staleness on the hottest keys. Switch to expireAfterWrite so a lost invalidation degrades to bounded staleness of at most expireTimeInMs, and document that bound. Claude-Session: https://claude.ai/code/session_015v8chvQJLYFBuBv1MiHebo --- .../main/java/org/apache/gravitino/Configs.java | 2 +- .../gravitino/cache/CaffeineEntityCache.java | 7 ++- .../cache/TestCaffeineEntityCacheExpiration.java | 68 ++++++++++++++++++++++ docs/gravitino-server-config.md | 5 +- 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/Configs.java b/core/src/main/java/org/apache/gravitino/Configs.java index c638288c39..5c854a786c 100644 --- a/core/src/main/java/org/apache/gravitino/Configs.java +++ b/core/src/main/java/org/apache/gravitino/Configs.java @@ -493,7 +493,7 @@ public class Configs { public static final ConfigEntry<Long> CACHE_EXPIRATION_TIME = new ConfigBuilder("gravitino.cache.expireTimeInMs") .doc( - "Time-to-live (TTL) for each cache entry after it is written, in milliseconds." + "Time-to-live (TTL) for each cache entry after it is written, in milliseconds. " + "Default is 3,600,000 ms (1 hour).") .version(ConfigConstants.VERSION_1_0_0) .longConf() diff --git a/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java b/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java index 2f79394dbc..0e117a3562 100644 --- a/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java +++ b/core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java @@ -335,7 +335,12 @@ public class CaffeineEntityCache extends BaseEntityCache { } if (cacheConfig.get(Configs.CACHE_EXPIRATION_TIME) > 0) { - builder.expireAfterAccess( + // Expire after write, not after access. The TTL is the safety net for a cross-node + // invalidation that never arrives (a lost entity_change_log row, a stalled poller). With an + // access-based TTL a stale entry that keeps being read would never expire, so a single missed + // invalidation would become permanent on exactly the hottest keys. A write-based TTL bounds + // that staleness to expireTimeInMs. + builder.expireAfterWrite( cacheConfig.get(Configs.CACHE_EXPIRATION_TIME), TimeUnit.MILLISECONDS); } diff --git a/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheExpiration.java b/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheExpiration.java new file mode 100644 index 0000000000..6f451bfe12 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/cache/TestCaffeineEntityCacheExpiration.java @@ -0,0 +1,68 @@ +/* + * 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.gravitino.cache; + +import com.github.benmanes.caffeine.cache.Policy; +import java.time.Duration; +import java.util.concurrent.TimeUnit; +import org.apache.gravitino.Config; +import org.apache.gravitino.Configs; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +/** + * Tests the expiration policy of {@link CaffeineEntityCache}. + * + * <p>The cache must expire entries a fixed time after they were written, never after they were last + * read. In a multi-node deployment the TTL is the safety net for a cross-node invalidation that + * never arrives; an access-based TTL would keep a hot stale entry alive forever. + */ +public class TestCaffeineEntityCacheExpiration { + + @Test + void testExpiresAfterWriteNotAfterAccess() { + Config config = new Config() {}; + config.set(Configs.CACHE_EXPIRATION_TIME, 600_000L); + + CaffeineEntityCache cache = new CaffeineEntityCache(config); + Policy<EntityCacheKey, ?> policy = cache.getCacheData().policy(); + + Assertions.assertTrue(policy.expireAfterWrite().isPresent()); + Assertions.assertEquals( + Duration.ofMillis(600_000L), policy.expireAfterWrite().get().getExpiresAfter()); + Assertions.assertFalse( + policy.expireAfterAccess().isPresent(), + "reads must not extend the lifetime of an entry: a stale entry that keeps being read " + + "would otherwise never expire"); + Assertions.assertEquals( + 600_000L, policy.expireAfterWrite().get().getExpiresAfter(TimeUnit.MILLISECONDS)); + } + + @Test + void testZeroExpirationDisablesTimeBasedEviction() { + Config config = new Config() {}; + config.set(Configs.CACHE_EXPIRATION_TIME, 0L); + + CaffeineEntityCache cache = new CaffeineEntityCache(config); + Policy<EntityCacheKey, ?> policy = cache.getCacheData().policy(); + + Assertions.assertFalse(policy.expireAfterWrite().isPresent()); + Assertions.assertFalse(policy.expireAfterAccess().isPresent()); + } +} diff --git a/docs/gravitino-server-config.md b/docs/gravitino-server-config.md index 7da4f85cfe..6c36190c80 100644 --- a/docs/gravitino-server-config.md +++ b/docs/gravitino-server-config.md @@ -301,7 +301,10 @@ by default, and the properties below tune what it holds and how it evicts. | `gravitino.cache.lockSegments` | Number of lock segments used to reduce contention. | `16` | Two eviction limits apply at once. Time to live always applies: an entry older than -`expireTimeInMs` expires and is cleaned up asynchronously. Alongside it, the cache bounds its size +`expireTimeInMs` expires and is cleaned up asynchronously. The clock starts when the entry is +written and is not reset by reads, so in a multi-node deployment `expireTimeInMs` is also the upper +bound on how long a node can serve a stale entry if a cross-node invalidation is ever missed (see +[Change Log Propagation](#change-log-propagation)). Alongside it, the cache bounds its size either by count or by weight. With `enableWeigher` disabled, Caffeine's W-TinyLFU policy evicts the least-used entries once `maxEntries` is reached. With `enableWeigher` enabled, each entity type carries a weight, larger for entities higher in the hierarchy, and eviction targets a total weight
