Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document ConfigurationPropertyCaching #34172

Closed
jizhuozhi opened this issue Feb 13, 2023 · 4 comments
Closed

Document ConfigurationPropertyCaching #34172

jizhuozhi opened this issue Feb 13, 2023 · 4 comments
Labels
type: documentation A documentation update
Milestone

Comments

@jizhuozhi
Copy link

jizhuozhi commented Feb 13, 2023

spring-boot version: 3.0.2 (since 2.3.0)

In commit 7afd25f, a cache was introduced to optimize the overhead caused by copying and deduplication of getPropertyNames objects.

But currently we have encountered some problems related to memory pressure. In the process of following the call stack, we found that the cache created in SpringIterableConfigurationPropertySource only exists in two states:

  1. Never expires
  2. Refresh forever

This is because there is no configuration in the SpringIterableConfigurationPropertySource, and there is no way to configure it (programmatic API or configuration file).

@philwebb
Copy link
Member

philwebb commented Feb 14, 2023

Are you able to provide some more details about the problems you're facing? Do you have very large property sources or is there some other reason why the memory pressure happens? Perhaps you can share a sample application that replicates the problem.

The cache states are currently set based on if the property source is immutable. The assumption being that if the property source can't change, it's not a problem to keep the cache.

...there is no way to configure it (programmatic API or configuration file

Have you seen the ConfigurationPropertyCaching interface? You should be able to do something like this to configure things:

ConfigurationPropertyCaching.get(sources, propertySource).setTimeToLive(timeToLive);

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Feb 14, 2023
@jizhuozhi
Copy link
Author

jizhuozhi commented Feb 14, 2023

Hello, @philwebb . Thanks for your reply :)

The actual usage scenario is a rules engine that relies on several keys in dynamic configuration, here is an example

        String[] keys = new String[]{ "foo", "bar", "foobar" /* other elements... */};
        String[] values = Arrays.stream(keys).map(environment::getProperty).toArray(String[]::new);
        /* do something with values... */

Different rules consume different keys. The complete data collection contains tens of thousands of keys and has a high QPS.

At the same time, unfortunately, we are using G1's immature JDK version, and due to enterprise restrictions, we cannot upgrade it. Therefore, the young generation garbage collection algorithm is still Parallel New.

Have you seen the ConfigurationPropertyCaching interface? You should be able to do something like this to configure things:

I did browse a lot of classes before this, but I paid too much attention to the SpringIterableConfigurationPropertySource class itself, which is package-private, but did not find the content related to ConfigurationPropertyCaching, which really solved my problem, but due to the lack of corresponding documents ( Including javadoc and various search engines) to introduce this way, I think other users may also encounter the same problem.

Just a suggestion, would it be very valuable to provide a user-accessible documentation.

@jizhuozhi
Copy link
Author

jizhuozhi commented Feb 15, 2023

Hello, @philwebb .

There is still one point that I want to confirm: whether caching should be enabled by default.

In the projects I have been in contact with, and many blogs have mentioned that properties can be obtained programmatically through Environment#getProperty. Unfortunately, no one has ever mentioned that you need to configure the cache yourself. Everyone has made an assumption: after getting the latest snapshot from the configuration server, we can get properties in O(1) as a hash table.

If this values can be cached by default (even if it is only 50ms, P99 of latency in our pure calculation scenario), it will also be greatly improved in the above scenario of cyclically obtaining values.

@philwebb philwebb added for: team-attention An issue we'd like other members of the team to review for: team-meeting An issue we'd like to discuss as a team to make progress and removed for: team-attention An issue we'd like other members of the team to review labels Feb 15, 2023
@philwebb philwebb changed the title The cache of SpringIterableConfigurationPropertySource is not enabled when using mutable property source The cache of SpringIterableConfigurationPropertySource is not enabled when using mutable property source Jul 19, 2023
@wilkinsona
Copy link
Member

We've discussed this today. Our feeling is that having 10s of thousands of properties is a fairly extreme edge case. We're also reluctant to enable caching by default as it will cause unexpected and hard to diagnose behavior when a mutable property source changes.

We'll update the reference documentation to make a brief mention of the ConfigurationPropertyCaching interface.

@wilkinsona wilkinsona added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided for: team-meeting An issue we'd like to discuss as a team to make progress labels Jul 20, 2023
@wilkinsona wilkinsona added this to the 2.7.x milestone Jul 20, 2023
@wilkinsona wilkinsona changed the title The cache of SpringIterableConfigurationPropertySource is not enabled when using mutable property source Document ConfigurationPropertyCaching Jul 20, 2023
@wilkinsona wilkinsona modified the milestones: 2.7.x, 2.7.18 Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

4 participants