dimas-b commented on code in PR #1339:
URL: https://github.com/apache/polaris/pull/1339#discussion_r2033751485
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCache.java:
##########
@@ -18,185 +18,74 @@
*/
package org.apache.polaris.core.persistence.cache;
-import com.github.benmanes.caffeine.cache.Cache;
-import com.github.benmanes.caffeine.cache.Caffeine;
-import com.github.benmanes.caffeine.cache.RemovalListener;
+import com.google.common.util.concurrent.Striped;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
-import java.util.AbstractMap;
-import java.util.List;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
+import java.util.Comparator;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.Lock;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.entity.PolarisBaseEntity;
import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.core.entity.PolarisGrantRecord;
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
import org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
/** The entity cache, can be private or shared */
public class EntityCache {
+ private static final Comparator<ResolvedPolarisEntity>
RESOLVED_POLARIS_ENTITY_COMPARATOR =
+ Comparator.nullsLast(
+ Comparator.<ResolvedPolarisEntity>comparingInt(rpe ->
rpe.getEntity().getEntityVersion())
+ .thenComparingInt(rpe ->
rpe.getEntity().getGrantRecordsVersion()));
+ private static final int STRIPES = 1_024;
// cache mode
private EntityCacheMode cacheMode;
// the meta store manager
private final PolarisMetaStoreManager polarisMetaStoreManager;
- // Caffeine cache to keep entries by id
- private final Cache<Long, ResolvedPolarisEntity> byId;
+ // Caffeine cache to keep entries
+ private final IndexedCache<CacheKey, ResolvedPolarisEntity> cache;
- // index by name
- private final AbstractMap<EntityCacheByNameKey, ResolvedPolarisEntity>
byName;
+ // Locks to ensure that an entity can only be refreshed by one thread at a
time
+ private final Striped<Lock> locks;
/**
* Constructor. Cache can be private or shared
*
* @param polarisMetaStoreManager the meta store manager implementation
*/
public EntityCache(@Nonnull PolarisMetaStoreManager polarisMetaStoreManager)
{
-
- // by name cache
- this.byName = new ConcurrentHashMap<>();
-
- // When an entry is removed, we simply remove it from the byName map
- RemovalListener<Long, ResolvedPolarisEntity> removalListener =
- (key, value, cause) -> {
- if (value != null) {
- // compute name key
- EntityCacheByNameKey nameKey = new
EntityCacheByNameKey(value.getEntity());
-
- // if it is still active, remove it from the name key
- this.byName.remove(nameKey, value);
- }
- };
-
- // use a Caffeine cache to purge entries when those have not been used for
a long time.
- // Assuming 1KB per entry, 100K entries is about 100MB.
- this.byId =
- Caffeine.newBuilder()
- .maximumSize(100_000) // Set maximum size to 100,000 elements
- .expireAfterAccess(1, TimeUnit.HOURS) // Expire entries after 1
hour of no access
- .removalListener(removalListener) // Set the removal listener
- .build();
-
// remember the meta store manager
this.polarisMetaStoreManager = polarisMetaStoreManager;
// enabled by default
this.cacheMode = EntityCacheMode.ENABLE;
+
+ // Use a Caffeine cache to purge entries when those have not been used for
a long time.
+ // Assuming 1KB per entry, 100K entries is about 100MB. Note that each
entity is stored twice in
+ // the cache, once indexed by its identifier and once indexed by its name.
+ this.cache =
+ new IndexedCache.Builder<CacheKey, ResolvedPolarisEntity>()
+ .primaryKey(e -> new IdKey(e.getEntity().getId()))
+ .addSecondaryKey(e -> new NameKey(new
EntityCacheByNameKey(e.getEntity())))
+ .expireAfterWrite(Duration.ofMinutes(5))
+ .maximumSize(10_000)
Review Comment:
Since the expiry limits and general cache behaviour is going to be different
from old code... Should be make these parameters configurable immediately? User
may need to adjust their cache config, I guess :sweat_smile:
--
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]
