[#9635] fix(core): fix tag association problem #9638
Conversation
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix a cache invalidation issue for tag associations where bidirectional relationships were not being properly invalidated. When associating tags with metadata objects, only one side of the relationship cache was being cleared, leading to stale cache data.
- Adds bidirectional cache invalidation in
insertRelationmethod (line 256) - Adds cache invalidation for destination entities in
updateEntityRelationsmethod (lines 269-275) - Includes a new comprehensive test for multiple tag bindings to verify the fix
However, the implementation contains a critical bug where the wrong entity type is used for destination entity cache invalidation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java | Adds cache invalidation for both source and destination entities in relation operations, but uses incorrect entity type for destinations |
| core/src/test/java/org/apache/gravitino/storage/TestEntityStorage.java | Adds new test case verifying that a single tag can be associated with multiple catalogs and the associations are correctly maintained |
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java
Show resolved
Hide resolved
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java
Show resolved
Hide resolved
|
|
||
| @ParameterizedTest | ||
| @MethodSource("storageProvider") | ||
| void testTagRelationMultipleBindings(String type) throws Exception { |
There was a problem hiding this comment.
Shall we split the cache-related tests to a new class? This file is too big to maintain.
There was a problem hiding this comment.
I would suggest not adding tests to this file continuously.
There was a problem hiding this comment.
I see and will try to use a separate class to hold them.
core/src/main/java/org/apache/gravitino/storage/relational/RelationalEntityStore.java
Show resolved
Hide resolved
2 participants
What changes were proposed in this pull request?
This pull request enhances the correctness and robustness of relation caching in the entity storage layer, and expands test coverage to ensure proper handling of multiple bindings for tags and policies. The main changes include improved cache invalidation logic in the
RelationalEntityStoreand new parameterized tests that verify relation consistency when entities are bound to multiple tags or policies.Why are the changes needed?
It's a bug
Fix: #9635
Does this PR introduce any user-facing change?
N/A.
How was this patch tested?
UTs.