eric-maynard commented on code in PR #490:
URL: https://github.com/apache/polaris/pull/490#discussion_r1861191589
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/cache/EntityCache.java:
##########
@@ -71,12 +71,13 @@ public EntityCache(@NotNull PolarisRemoteCache
polarisRemoteCache) {
};
// 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
+ .maximumWeight(100 * EntityWeigher.WEIGHT_PER_MB) // Goal is ~100MB
Review Comment:
This is a good callout. My concern with making it configurable is that the
limit is close to arbitrary.
As you noted in another comment, we have a "goal" of 100MB but that could
really be 200MB or 1MB depending on how eviction happens with soft values and
the weights. So while raising the limit definitely gets you a bigger cache,
it's quite opaque as to what value you would actually want to set.
Also, the EntityCache is ideally very transparent and not something a
typical end user would want to concern themselves with.
In light of the above I kept it hard-coded for now but if you feel strongly
that we should make it a `featureConfiguration` I am comfortable doing so
--
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]