Skip to content

Commit bb6b039

Browse files
authored
[#12151] improvement(core): complete cached entity change-log coverage (#12374)
Depends on #12111. ### What changes were proposed in this pull request? Centralize cache invalidation change-log emission at the `JDBCBackend` entity-store mutation boundary. - Emit `ALTER` for cacheable entity overwrite and update operations. - Emit `DROP` after a successful delete. - Keep only the old identifier for rename invalidation. - Do not emit an event for create because the entity cache has no negative entries and list operations bypass the cache. - Emit one root `DROP` for cascading deletion and let prefix invalidation clear cached descendants. - Commit the metadata mutation and its change-log row in the same transaction. - Remove duplicated change-log emission from type-specific MetaServices. - Update the multi-node cache design and mutation coverage tests. The covered cacheable types are metalake, catalog, schema, table, topic, view, fileset, tag, policy, and job. ### Why are the changes needed? Change-log emission was distributed across selected MetaService paths. This missed ordinary non-rename updates and mutation paths such as overwrite, status changes, enable/disable, import, and repair. Centralizing emission ensures that every invalidating mutation of a cacheable entity follows the same transaction and coverage policy. Fix: #12151 ### Does this PR introduce _any_ user-facing change? No. This PR does not change public APIs or configuration properties. ### How was this patch tested? - `./gradlew :core:spotlessApply -PskipDockerTests` - `./gradlew :core:test --tests "org.apache.gravitino.storage.relational.service.TestEntityChangeLogService" -PskipITs -PskipDockerTests` - `./gradlew :core:test --tests "org.apache.gravitino.storage.relational.service.TestTableMetaService" -PskipITs -PskipDockerTests` - `./gradlew :core:test --tests "org.apache.gravitino.storage.relational.TestEntityCacheCrossNodeInvalidation" -PskipITs -PskipDockerTests` - `./gradlew :core:test -PskipITs -PskipDockerTests` - Added `TestEntityStorageChangeLog`, which runs the mutation lifecycle and cascade-drop assertions against H2, MySQL, and PostgreSQL with cache enabled and disabled.
1 parent 9951a4d commit bb6b039

39 files changed

Lines changed: 1870 additions & 562 deletions

core/src/main/java/org/apache/gravitino/cache/BaseEntityCache.java

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,35 +45,25 @@ public abstract class BaseEntityCache implements EntityCache {
4545
* SupportsEntityStoreCache#put(Entity)} for the contract. New entity types are excluded by
4646
* default until their invalidation behavior has been validated.
4747
*
48-
* <p>{@code USER}, {@code GROUP} and {@code ROLE} are materialized with relation-derived data
49-
* joined in at load time: a role carries its securable objects, and a user/group carries its role
50-
* names. A mutation on the entity itself invalidates its own key through the write path, but this
51-
* embedded data also goes stale through a mutation on a different entity. For example, deleting
52-
* or renaming a securable object changes a role's materialized form, and deleting or renaming a
53-
* role changes a user's/group's role names. Such a mutation touches neither this entity's own key
54-
* nor any hierarchy ancestor of it, so neither the write-path invalidation nor a prefix cascade
55-
* over the entity hierarchy would evict it. Caching them would therefore serve stale
56-
* authorization data.
48+
* <p>Only self-contained entities are cacheable: a stale copy of one is at worst cosmetically old
49+
* (an old comment, property, or job status), never a wrong pointer, and each can be invalidated
50+
* with a single one-to-one key drop. Every other type is read straight from the store.
51+
* User/group/role embed relation-derived data that a per-node cache cannot invalidate;
52+
* model/model version and function carry a load-bearing pointer that would be silently wrong if
53+
* served stale.
5754
*/
5855
private static final Set<Entity.EntityType> CACHEABLE_TYPES =
5956
Sets.immutableEnumSet(
6057
Entity.EntityType.METALAKE,
6158
Entity.EntityType.CATALOG,
6259
Entity.EntityType.SCHEMA,
6360
Entity.EntityType.TABLE,
61+
Entity.EntityType.TOPIC,
6462
Entity.EntityType.VIEW,
65-
Entity.EntityType.COLUMN,
6663
Entity.EntityType.FILESET,
67-
Entity.EntityType.TOPIC,
6864
Entity.EntityType.TAG,
69-
Entity.EntityType.MODEL,
70-
Entity.EntityType.MODEL_VERSION,
7165
Entity.EntityType.POLICY,
72-
Entity.EntityType.TABLE_STATISTIC,
73-
Entity.EntityType.JOB_TEMPLATE,
74-
Entity.EntityType.JOB,
75-
Entity.EntityType.AUDIT,
76-
Entity.EntityType.FUNCTION);
66+
Entity.EntityType.JOB);
7767

7868
protected final Config cacheConfig;
7969

@@ -88,6 +78,18 @@ public BaseEntityCache(Config config) {
8878
this.cacheConfig = config;
8979
}
9080

81+
/**
82+
* {@inheritDoc}
83+
*
84+
* <p>Defaults to {@link Coherence#LOCAL_PER_NODE}: an in-memory, per-node cache whose changes
85+
* must be propagated to the other nodes. A shared implementation must override this to return
86+
* {@link Coherence#SHARED}.
87+
*/
88+
@Override
89+
public Coherence coherence() {
90+
return Coherence.LOCAL_PER_NODE;
91+
}
92+
9193
/**
9294
* Returns whether entities of the given type may be cached.
9395
*

core/src/main/java/org/apache/gravitino/cache/CaffeineEntityCache.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import com.googlecode.concurrenttrees.radix.RadixTree;
3131
import com.googlecode.concurrenttrees.radix.node.concrete.DefaultCharArrayNodeFactory;
3232
import java.util.List;
33-
import java.util.Objects;
3433
import java.util.Optional;
3534
import java.util.concurrent.ArrayBlockingQueue;
3635
import java.util.concurrent.ExecutorService;
@@ -43,7 +42,6 @@
4342
import org.apache.gravitino.Entity;
4443
import org.apache.gravitino.HasIdentifier;
4544
import org.apache.gravitino.NameIdentifier;
46-
import org.apache.gravitino.meta.ModelVersionEntity;
4745
import org.apache.gravitino.utils.HierarchicalSchemaUtil;
4846
import org.slf4j.Logger;
4947
import org.slf4j.LoggerFactory;
@@ -57,10 +55,10 @@
5755
* schemas and tables under it).
5856
*
5957
* <p>Relation query results are NOT cached by this implementation; relation and list operations
60-
* always fall back to the {@code EntityStore}. Entity types whose materialized form embeds
61-
* relation-derived data ({@code USER}, {@code GROUP}, {@code ROLE}) are excluded from caching
62-
* entirely by {@link BaseEntityCache#put}, because without relation tracking their entries could
63-
* not be invalidated when the referenced entities change.
58+
* always fall back to the {@code EntityStore}. Only the self-contained metadata objects listed in
59+
* {@link BaseEntityCache#isCacheable} are cached; every other type (user/group/role, model/model
60+
* version, function, job template, and any type not in that allowlist) is read straight from the
61+
* {@code EntityStore}.
6462
*/
6563
public class CaffeineEntityCache extends BaseEntityCache {
6664
private static final int CACHE_CLEANUP_CORE_THREADS = 1;
@@ -228,13 +226,9 @@ protected <E extends Entity & HasIdentifier> void doPut(E entity) {
228226
/** {@inheritDoc} */
229227
@Override
230228
public <E extends Entity & HasIdentifier> void invalidateOnKeyChange(E entity) {
231-
// Invalidate the cache if inserting the entity may affect related cache keys.
232-
// For example, inserting a model version changes the latest version of the model,
233-
// so the corresponding model cache entry should be invalidated.
234-
if (Objects.requireNonNull(entity.type()) == Entity.EntityType.MODEL_VERSION) {
235-
NameIdentifier modelIdent = ((ModelVersionEntity) entity).modelIdentifier();
236-
invalidate(modelIdent, Entity.EntityType.MODEL);
237-
}
229+
// Every cacheable entity is self-contained (see BaseEntityCache#isCacheable), so inserting one
230+
// never requires invalidating a different key. Kept for the SPI contract; implementations that
231+
// cache derived entries can override this.
238232
}
239233

240234
/** {@inheritDoc} */
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.gravitino.cache;
21+
22+
/**
23+
* Describes how an {@link EntityCache} implementation stays coherent across a multi-node Gravitino
24+
* cluster. The write path reads this marker to decide whether a change made on one node must be
25+
* propagated to the other nodes.
26+
*/
27+
public enum Coherence {
28+
/** The implementation does not retain entries, so it needs no coherence mechanism. */
29+
NONE,
30+
31+
/**
32+
* Each node keeps its own copy of the cache, so a change made on one node does not clear the
33+
* copies held by the other nodes. To stay correct across a cluster the write path must publish
34+
* the change (via the {@code entity_change_log}) so every other node's poller invalidates its own
35+
* copy. The default {@code caffeine} cache is {@code LOCAL_PER_NODE}.
36+
*/
37+
LOCAL_PER_NODE,
38+
39+
/**
40+
* A single copy is shared by the whole cluster, so a write clears it once and every node observes
41+
* the change immediately. There is nothing per-node to propagate. The {@code redis} cache is
42+
* {@code SHARED}.
43+
*/
44+
SHARED
45+
}

core/src/main/java/org/apache/gravitino/cache/EntityCache.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@
2828
* this SPI; relation and list operations always fall back to the {@code EntityStore}.
2929
*/
3030
public interface EntityCache extends SupportsEntityStoreCache {
31+
/**
32+
* Returns how this cache stays coherent across a multi-node cluster. The write path uses this to
33+
* decide whether a local change must be propagated to the other nodes (see {@link Coherence}).
34+
*
35+
* @return the {@link Coherence} model of this cache
36+
*/
37+
Coherence coherence();
38+
3139
/**
3240
* Clears all entries from the cache, including data and index, resetting it to an empty state.
3341
*/

core/src/main/java/org/apache/gravitino/cache/NoOpsCache.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ public NoOpsCache(Config config) {
3737
super(config);
3838
}
3939

40+
/** {@inheritDoc} */
41+
@Override
42+
public Coherence coherence() {
43+
return Coherence.NONE;
44+
}
45+
4046
/** {@inheritDoc} */
4147
@Override
4248
protected void invalidateExpiredItem(EntityCacheKey key) {

core/src/main/java/org/apache/gravitino/cache/SupportsEntityStoreCache.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,11 @@ <E extends Entity & HasIdentifier> Optional<E> getIfPresent(
6868
* invalidation path covers, so it must remain non-cacheable until its invalidation behavior has
6969
* been validated. Implementations extending {@link BaseEntityCache} get this check for free.
7070
*
71-
* <p>{@code USER}, {@code GROUP} and {@code ROLE} are deliberately excluded because they are
72-
* materialized with relation-derived data joined in at load time (a role carries its securable
73-
* objects, and a user/group carries its role names). That embedded data goes stale through
74-
* mutations on other entities that touch neither this entity's own key nor any of its hierarchy
75-
* ancestors. Since no invalidation path covers them, caching them serves stale authorization
76-
* data.
71+
* <p>Only self-contained entities are approved. User/group/role are deliberately excluded because
72+
* they contain relation-derived data whose source can change through another entity. Model/model
73+
* version and function are also excluded because they carry load-bearing pointers that must not
74+
* be served stale. Other unapproved and newly introduced entity types go straight to the store
75+
* until their invalidation behavior has been validated.
7776
*
7877
* <p>Implementations must also invoke {@link #invalidateOnKeyChange(Entity)} for every entity,
7978
* including non-cacheable ones, since a non-cacheable entity may still invalidate a cacheable

core/src/main/java/org/apache/gravitino/catalog/CatalogChangeLogListener.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.gravitino.Entity.EntityType;
2525
import org.apache.gravitino.NameIdentifier;
2626
import org.apache.gravitino.storage.relational.EntityChangeLogListener;
27+
import org.apache.gravitino.storage.relational.EntityChangeLogNameIdentifierCodec;
2728
import org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord;
2829
import org.slf4j.Logger;
2930
import org.slf4j.LoggerFactory;
@@ -116,11 +117,17 @@ private Optional<NameIdentifier> catalogIdentifier(EntityChangeRecord change) {
116117
return Optional.empty();
117118
}
118119

119-
String[] names = change.getFullName().split("\\.");
120-
if (names.length != 2) {
120+
NameIdentifier ident;
121+
try {
122+
ident = EntityChangeLogNameIdentifierCodec.decode(change.getFullName());
123+
} catch (IllegalArgumentException e) {
121124
LOG.warn("Invalid catalog full name in entity change log: {}", change.getFullName());
122125
return Optional.empty();
123126
}
124-
return Optional.of(NameIdentifier.of(names[0], names[1]));
127+
if (ident.namespace().length() != 1) {
128+
LOG.warn("Invalid catalog full name in entity change log: {}", change.getFullName());
129+
return Optional.empty();
130+
}
131+
return Optional.of(ident);
125132
}
126133
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.gravitino.storage.relational;
20+
21+
import com.google.common.base.Preconditions;
22+
import java.util.List;
23+
import java.util.Locale;
24+
import org.apache.gravitino.Entity.EntityType;
25+
import org.apache.gravitino.NameIdentifier;
26+
import org.apache.gravitino.cache.EntityCache;
27+
import org.apache.gravitino.storage.relational.po.cache.EntityChangeRecord;
28+
import org.slf4j.Logger;
29+
import org.slf4j.LoggerFactory;
30+
31+
/**
32+
* Keeps a per-node {@link EntityCache} coherent across a multi-node cluster by replaying {@code
33+
* entity_change_log} rows written by other nodes.
34+
*
35+
* <p>Every ALTER/DROP row is replayed as a direct {@link EntityCache#invalidate(NameIdentifier,
36+
* EntityType)} for exactly the changed entity key. Because the cache indexes its keys by identifier
37+
* prefix, invalidating a container (for example a schema) cascades to its cached children on the
38+
* local node through that forward prefix scan; no reverse index is involved.
39+
*
40+
* <p>This listener is registered only for a {@link
41+
* org.apache.gravitino.cache.Coherence#LOCAL_PER_NODE} cache: a shared cache has a single
42+
* cluster-wide copy and nothing per-node to invalidate. It is called <em>synchronously</em> on the
43+
* poller thread, so it performs only fast, in-memory, idempotent invalidations.
44+
*
45+
* <p>The two failure modes of a replayed row are handled separately, because they mean different
46+
* things:
47+
*
48+
* <ul>
49+
* <li>A <b>malformed row</b> (unknown entity type, undecodable full name) names no entity, so
50+
* there is nothing to invalidate. It is logged and skipped, and the rest of the batch still
51+
* applies.
52+
* <li>A <b>failed invalidation</b> means this node may now serve stale metadata indefinitely. The
53+
* whole cache is cleared instead, which is strictly stronger than the invalidation that
54+
* failed and only costs a cold-cache penalty, since the cache is derived state. If even the
55+
* clear fails the exception propagates, and {@link EntityChangeLogPoller} retries the batch
56+
* and ultimately applies its configured listener failure action.
57+
* </ul>
58+
*/
59+
public class EntityCacheChangeLogListener implements EntityChangeLogListener {
60+
61+
private static final Logger LOG = LoggerFactory.getLogger(EntityCacheChangeLogListener.class);
62+
63+
private final EntityCache cache;
64+
65+
/**
66+
* Creates a listener that invalidates the given entity store cache.
67+
*
68+
* @param cache the per-node entity store cache to keep coherent
69+
*/
70+
public EntityCacheChangeLogListener(EntityCache cache) {
71+
Preconditions.checkArgument(cache != null, "cache cannot be null");
72+
this.cache = cache;
73+
}
74+
75+
@Override
76+
public void onEntityChange(List<EntityChangeRecord> changes) {
77+
for (EntityChangeRecord change : changes) {
78+
EntityType type = entityType(change);
79+
NameIdentifier ident = identifier(change);
80+
if (type == null || ident == null) {
81+
// Already logged by the parsing helpers. A row that names no entity cannot invalidate
82+
// anything, so skipping it leaves no stale entry behind.
83+
continue;
84+
}
85+
86+
try {
87+
LOG.debug("Invalidating entity cache due to entity change log: {} ({})", ident, type);
88+
cache.invalidate(ident, type);
89+
} catch (RuntimeException e) {
90+
// Dropping a single invalidation would leave this node serving that entity stale until it
91+
// expires. Clearing the whole cache is the safe superset, and it also covers the rest of
92+
// this batch, so there is nothing left to replay.
93+
LOG.error(
94+
"Failed to invalidate {} ({}) from the entity change log, clearing the local entity "
95+
+ "cache to stay coherent",
96+
ident,
97+
type,
98+
e);
99+
cache.clear();
100+
return;
101+
}
102+
}
103+
}
104+
105+
private EntityType entityType(EntityChangeRecord change) {
106+
if (change.getEntityType() == null) {
107+
LOG.warn("Invalid entity type in entity change log: null");
108+
return null;
109+
}
110+
try {
111+
return EntityType.valueOf(change.getEntityType().toUpperCase(Locale.ROOT));
112+
} catch (IllegalArgumentException e) {
113+
LOG.warn("Unknown entity type in entity change log: {}", change.getEntityType());
114+
return null;
115+
}
116+
}
117+
118+
private NameIdentifier identifier(EntityChangeRecord change) {
119+
String fullName = change.getFullName();
120+
if (fullName == null || fullName.isEmpty()) {
121+
LOG.warn("Invalid full name in entity change log: {}", fullName);
122+
return null;
123+
}
124+
try {
125+
return EntityChangeLogNameIdentifierCodec.decode(fullName);
126+
} catch (IllegalArgumentException e) {
127+
LOG.warn("Undecodable full name in entity change log: {}", fullName, e);
128+
return null;
129+
}
130+
}
131+
}

0 commit comments

Comments
 (0)