Copilot commented on code in PR #12006:
URL: https://github.com/apache/gravitino/pull/12006#discussion_r3689436100
##########
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java:
##########
@@ -471,20 +392,19 @@ public <E extends Entity & HasIdentifier> List<E>
updateEntityRelations(
NameIdentifier[] destEntitiesToAdd,
NameIdentifier[] destEntitiesToRemove)
throws IOException, NoSuchEntityException, EntityAlreadyExistsException {
-
- // Invalidate after the backend write, not before. Invalidating before
creates a window where
- // a concurrent read can repopulate the cache with stale pre-commit data.
List<E> result =
backend.updateEntityRelations(
relType, srcEntityIdent, srcEntityType, destEntitiesToAdd,
destEntitiesToRemove);
- cache.invalidate(srcEntityIdent, srcEntityType, relType);
+ // Invalidate after the backend write, not before: invalidating first
opens a window where a
+ // concurrent read could repopulate the cache with stale pre-commit data.
+ cache.invalidate(srcEntityIdent, srcEntityType);
for (NameIdentifier destToAdd : destEntitiesToAdd) {
- cache.invalidate(destToAdd, srcEntityType, relType);
+ cache.invalidate(destToAdd, srcEntityType);
}
Review Comment:
In updateEntityRelations(), destination identifiers are invalidated using
srcEntityType. For TAG_METADATA_OBJECT_REL / POLICY_METADATA_OBJECT_REL this
means tag/policy cache entries are never invalidated (because they are cached
under EntityType.TAG/POLICY, not the source metadata object type). This breaks
the intended "invalidate endpoints" behavior for relation writes and can leave
stale tag/policy entities cached.
--
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]
