Skip to content

Replace ConcurrentHashMap with Caffine cache for configs#3599

Merged
EdColeman merged 1 commit intoapache:2.1from
EdColeman:ServerConfigurationFactory_update_caching
Jul 20, 2023
Merged

Replace ConcurrentHashMap with Caffine cache for configs#3599
EdColeman merged 1 commit intoapache:2.1from
EdColeman:ServerConfigurationFactory_update_caching

Conversation

@EdColeman
Copy link
Contributor

Using a cache instead of a map to ensure that configs are eventually cleaned up on deletions. The issue was raised in PR #3588

Using a cache instead of a map to ensures that configs are cleaned up on
deletions. The issues was rasied in PR apache#3588
@EdColeman EdColeman self-assigned this Jul 12, 2023
this.systemConfig = memoize(() -> new SystemConfiguration(context,
SystemPropKey.of(context.getInstanceID()), siteConfig));
tableParentConfigs =
Caffeine.newBuilder().expireAfterAccess(CACHE_EXPIRATION_HRS, TimeUnit.HOURS).build();
Copy link
Contributor

@keith-turner keith-turner Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if adding a weigher would make sense. Looking at the caffine docs the weight is only acquired when something is added to the cache. So it would not really handle the case of configuration growing in size (like a user adds a large prop after its in the cache). So a weigher probably does not makes sense for this case because Configuration objects have a dynamic size.

@EdColeman EdColeman merged commit 4c649e0 into apache:2.1 Jul 20, 2023
@EdColeman EdColeman deleted the ServerConfigurationFactory_update_caching branch July 20, 2023 22:41
@ctubbsii ctubbsii added this to the 2.1.2 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants