This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new dd774c3b3f [#12377] improvement(core): expire entity cache entries
after write, not after access (#13099)
dd774c3b3f is described below
commit dd774c3b3fd256638c896be78516e32260dfa553
Author: Qi Yu <[email protected]>
AuthorDate: Mon Sep 21 16:57:58 2026 +0800
[#12377] improvement(core): expire entity cache entries after write, not
after access (#13099)
### What changes were proposed in this pull request?
- `CaffeineEntityCache`: build the cache with `expireAfterWrite` instead
of `expireAfterAccess`.
- `Configs.CACHE_EXPIRATION_TIME`: fix a missing space in the doc
string.
- `docs/gravitino-server-config.md`: state that the TTL clock starts at
write time and is not reset by reads, so `expireTimeInMs` is also the
upper bound on stale reads in a multi-node deployment if a cross-node
invalidation is ever missed.
- New `TestCaffeineEntityCacheExpiration` asserting the Caffeine policy
is write-based (and absent when the TTL is `0`).
### Why are the changes needed?
`gravitino.cache.expireTimeInMs` and the docs both describe a TTL
measured from the write, but the implementation used
`expireAfterAccess`. With an access-based TTL, an entry that keeps being
read never expires, so a single missed cross-node invalidation (lost
`entity_change_log` row, stalled poller) becomes permanent staleness on
exactly the hottest keys. A write-based TTL bounds that to
`expireTimeInMs`.
This is item 1 of #12377; the other items are left for follow-ups.
Fix: #12377
### Does this PR introduce _any_ user-facing change?
Behaviour change only: entries now expire `expireTimeInMs` after they
were written, regardless of reads. Hot entries are reloaded from the DB
once per TTL (default 1 hour). No config keys added or removed.
### How was this patch tested?
`TestCaffeineEntityCacheExpiration` (new), plus the existing
`org.apache.gravitino.cache.*` and `TestEntityCache*` unit tests.
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 23fd879df1..de6e2315c6 100644
--- a/core/src/main/java/org/apache/gravitino/Configs.java
+++ b/core/src/main/java/org/apache/gravitino/Configs.java
@@ -494,7 +494,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 96422306c8..824d29ebf3 100644
--- a/docs/gravitino-server-config.md
+++ b/docs/gravitino-server-config.md
@@ -302,7 +302,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
