[#7177] feat(core): Add a Caffeine-based implementation for EntityCache

[Abyss-lord]

What changes were proposed in this pull request?

Fix some bugs in EntityCache and add test cases.

1. Move the database access logic to BaseEntityCache for better reuse and abstraction.
2. Add an allFields parameter to loadEntitiesByRelation to align with SupportsRelationOperations#listEntitiesByRelation.
3. Add an entityCache field to GravitinoEnv for direct access to the cache from components.

• Implement the methods related to the SupportsEntityStoreCache and SupportsRelationEntityCache interface in CaffeineEntityCache.
• Add unit tests for both index and cache.
• Use JCStress to perform multi-threaded testing on CaffeineEntityCache and the CacheIndex.

Category	Test Case Description	Method(s)
Basic Functionality	Basic put/get/getIfPresent operations	testPutAndGet, testGetIfPresent
	Same identifier with different entity types	testPutSameIdentifierEntities
	Size counting	testSize
	Clear the cache	testClear
Cache Loading	Hit from cache, no store access	testGetFromCache
	Miss from cache, fallback to store	testGetFromStore
	Reload after invalidation	testRemoveThenReload
Invalidation	Invalidate by METALAKE/CATALOG/SCHEMA/TABLE level	testInvalidate* series
	Invalidate non-existent entity	testRemoveNonExistentEntity
Relation Handling	Put/Get/Invalidate entity relations	Covered in multiple tests
	Fallback to listEntitiesByRelation from store	testGetFromStore
Eviction Policies	Expire by time	testExpireByTime
	Expire by size	testExpireBySize
	Expire by weight	testExpireByWeight
	Exceed max weight immediately	testExpireByWeightExceedMaxWeight
Weight Logic	Correctness of entity weight calculation	testWeightCalculation
Error & Boundary	Store throws exception	testLoadEntityThrowException
	Store is null	testNullEntityStore
	Null argument checks	test*WithNull methods
Helper/Validation	Identifier equality check	testEquals

Why are the changes needed?

Fix: #7177

Does this PR introduce any user-facing change?

no

How was this patch tested?

local test.

[Abyss-lord]

Hi @xunliu @jerryshao , could you please review this PR when you have time? I’d really appreciate your feedback.

[jerqi]

Is it necessary to have this parameter?

[Abyss-lord]

Yes, this parameter is necessary. In the context of listEntitiesByRelation, we need to fetch entities that are related to the given one, and passing true the method will fetch all the fields, Otherwise, the method will fetch all the fields except for high-cost fields.

[jerryshao]

As I remember correctly, the cache implementation is pluggable, so we should have a factory to create the cache interface. Here, we directly created a caffeine cache, I think we need to update the code.

Besides, should the cache be created after the entity store is initialized?

[jerryshao]

BTW, the entityCache will be null if the cache is not enabled. From my point, we should not create a null object deliberately, this will make it hard for the downstream developer to use this object, they have to check if this cache is null or not, and it is not a good implementation.

If the cache is not enabled, we should directly pass through to the entity store call. So for the users of this cache, they can directly use it without needing to add more check.

[Abyss-lord]

Thanks @jerryshao for the suggestion, makes sense!

I've updated the implementation to support pluggable cache via Java SPI. Specifically:

• Introduced a CacheFactory and CacheProvider interface to support dynamic creation of EntityCache implementations;
• Added a new configuration key gravitino.cache.typeName to specify the desired cache backend (e.g., "Caffeine");
• When cache is enabled, we instantiate the appropriate EntityCache via SPI and inject it into CachedEntityStore;
• When cache is disabled, we still create a CachedEntityStore, but pass null for the cache instance, so all operations fall through to the underlying store transparently.
  This way, downstream logic only needs to work with EntityStore and doesn't need to care whether caching is enabled or not — the behavior is internally consistent and clean.

[jerryshao]

I saw you already have a same lock in cache, and you're using the lock again in here. So is it necessary to have a lock in the cache class, since you will always have the lock here?

[Abyss-lord]

The internal contains(...) call doesn’t need to acquire the lock again. I’ll remove the redundant withLock(...) to simplify the locking.

[Abyss-lord]

Hi @jerryshao , I've completed the code updates and would appreciate your review of the PR when you have a moment.