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

ScheduledDataLoaderService should load from remote and ignore resource cache #366

Open
2kewl4u opened this issue Jul 28, 2021 · 15 comments
Open
Labels
Milestone

Comments

@2kewl4u
Copy link

2kewl4u commented Jul 28, 2021

The ScheduledDataLoaderService can be configured to schedule a TimerTask to load the LoadableResource and trigger the DefaultLoaderListener. These listeners will parse the data from the resource and update their internal caches with new ExchangeRate objects.

However, the scheduler will not load any new exchange rates and our data is most of the time outdated for a long time.

The reason is that there is actually a second level caching mechanism that prevents loading new data. The call to LoadableResource.load() will check if the resource has been loaded recently and in that case does not load from the remote source again.

There is a property 'cacheTTLMillis' which is by default

/**
The time to live (TTL) of cache entries in milliseconds, by default 24 h.
 */
private long cacheTTLMillis = 3600000L * 24; // 24 h

Luckily this can be disabled in the 'javamoney.properties' file.

Besides that easy fix, there is no reason to configure the scheduler to regularly load the resource if the cache will prevent this from happening. I suggest that the TimerTask within the ScheduledDataLoaderService always reads from the remote source (and fallback if desired). An alternative would be to adjust the default value of 'cacheTTLMillis' to be somewhat compatible with the 'period' property of the scheduler.

@keilw
Copy link
Member

keilw commented Jul 28, 2021

Do you feel checking once every 24h if the cached data is still there causes a performance problem or is too frequently?

@2kewl4u
Copy link
Author

2kewl4u commented Jul 28, 2021

Lets to an example. ECB publishes exchange rates at 16:00 CET. The scheduler is configured by default in version 1.4.2 with 03:00 hours period.

Assuming the process is started at 15:50 CET, the resource is loaded, but the exchange rate for the current day is not yet available, thus the resource returned is from the previous day. The scheduler will then 3 hours later at 18:50 try to check for an update. At least I thought that was the plan. But the scheduler will be blocked the next 24 hours by the internal cache of the LoadableResource and therefore won't update the exchange rate that was published at 16:00 until the next day. Depending on the time you start the process, it can be more or less.

You cannot control the behavior of the update with the scheduler unless the cache will be disabled or set to a very short amount that doesn't necessarily block the scheduler.

@keilw
Copy link
Member

keilw commented Jul 28, 2021

Of course ECB is only one example of a provider, others may update exchange rates more often.

The scheduler is configured by default in version 1.4.2 with 03:00 hours period.

Is that the generic fallback or the default for ECB?

@2kewl4u
Copy link
Author

2kewl4u commented Jul 28, 2021

I checked all ' javamoney.properties' files in
moneta-convert-ecb-1.4.2.jar
moneta-convert-imf-1.4.2.jar
none of them specifies the 'cacheTTLMillis', thus for all providers a cache of 24h applies.

And as you said, there might be providers who update more frequently, but the problem in my opinion stays the same. The ScheduledDataLoaderService task scheduler can only be configured correctly if the cache of the resource will be disabled or adjusted to the 'period' values. If the scheduler is configured to load the resource every 5 minutes, only once every 24 hours this will succeed.

@keilw
Copy link
Member

keilw commented Jul 28, 2021

I just checked the sources.
load.ECBHistoric90RateProvider.period=03:00
is for ECB, so maybe the default properties file could be higher, even close to the 24h if the actual refresh does not happen more than once a day by ECB.

There is a problem in the current snapshot of the MonetaryConfig mechanism as such, but aside from fixing that we don't have major plans to overhaul that before a Moneta and Money JSR 2.0 release.
We plan to use Jakarta Config for the next major version of Moneta and some of the SPI elements here are going to be deprecated in favor of the Config Spec, or already were, see MonetaryConfig.

@2kewl4u
Copy link
Author

2kewl4u commented Jul 28, 2021

I would suggest a simple fix: Instead of calling LoadableResource.load() within the TimerTask of the ScheduledDataLoaderService, simply call LoadableResource.loadRemote(). This will bypass the cache checkup and make sure that the scheduler can actually be configured to check for updates.

Otherwise I don't really understand what's the purpose of the scheduler here. It triggers the listeners even if the resource hasn't changed and therefore does parse the same data as before.

@keilw
Copy link
Member

keilw commented Jul 28, 2021

Would you have a PR for that simple and easy fix? While for the Spec (JSR 354 API) we usually require contributors to also join the JCP in the RI unless you are happy to join, we are a bit more flexible. Of over 30 or so contributors only around half are JCP members. If someone contributes often, then it is advisable.
More substantial changes are quite possible in a JavaMoney 2.0 release but first we would get rid of the internal SPI in favor of Jakarta Config.
The whole configuration subsystem was a "pet project" by @atsticks who also started a config project calle Apache Tamaya, but that was shelved by Apache Foundation and remained as an inspiration for Eclipse MicroProfile Config and ultimately Jakarta Config, so we plan to get rid of this "Configuration Light" in favor of a true configuration standard with the next major version of Moneta.

@2kewl4u
Copy link
Author

2kewl4u commented Jul 28, 2021

The only thing I understood was PR, thanks to google telling me that's a 'pull request'. I haven't contributed to any open source github projects yet and I'm not familiar with the java community process (JCP). I will have to read more to understand what it is about. Please don't take it personal if you haven't got a PR yet :)

@keilw
Copy link
Member

keilw commented Jul 28, 2021

No worries, thanks for the suggestion. I categorized the other one as ".Next" because if feels more like the next major version, while a few tweaks even changing the 3h to 13 or 23 could be done in the next minor version.

@biegeeinheit
Copy link

biegeeinheit commented Aug 17, 2021

We face the same problem.

How exactly can the parameter be overwritten in javamoney.properties for the short term fix?
cacheTTLMillis=0 did not work for us.

Thank you in advance.

@keilw
Copy link
Member

keilw commented Aug 17, 2021

@2kewl4u Any advice on that?

@2kewl4u
Copy link
Author

2kewl4u commented Aug 17, 2021

We face the same problem.

How exactly can the parameter be overwritten in javamoney.properties for the short term fix?
cacheTTLMillis=0 did not work for us.

Thank you in advance.

As of version 1.4.2 the class org.javamoney.moneta.spi.MonetaryConfig provides the static loading logic for the 'javamoney.properties' files. It assumes all those files on the classpath to have a priority prefix in front of each configuration entry or will assign the default of 0. If the property was already present in another 'javamoney.properties' file with the same priority, the loading fails with

new IllegalStateException("AmbiguousConfiguration detected for '" + key + "'.")

The properties are then used by the org.javamoney.moneta.internal.loader.LoaderConfigurator that searches for the prefix 'load.' and the suffix '.type'. Anything in between is considered to be the name of the resource. Then the org.javamoney.moneta.internal.loader.LoadableResource is created with properties from the 'javamoney.properties' that matches the prefix 'load.<resourceName>.', where this prefix is then omitted.

The org.javamoney.moneta.internal.loader.LoadableResource searches for an entry 'cacheTTLMillis' and tries to convert it into a long representing millis.

Long story short:
Create a 'javamoney.properties' file and place it into the classpath. Add an entry for the resource to be reconfigured. As an example, take the ECBCurrentRateProvider, the entry looks like this:

{0}load.ECBCurrentRateProvider.cacheTTLMillis=0

Add these entries for each resource name that you want to configure. If you have trouble with this, simply place a breakpoint into the constructor of the LoadableResource or the MonetaryConfig and check where your config got lost.

@biegeeinheit
Copy link

@2kewl4u Thank you for the detailed explainatory reply. It was really helpful.

@2kewl4u
Copy link
Author

2kewl4u commented Aug 30, 2021

@keilw hey i cloned the git repository and imported the projects from the master branch to try and provide some simple fixes for the current version as discussed. currently i fail to setup the project with maven error

Non-resolvable parent POM for org.javamoney:moneta-parent:1.4.3-SNAPSHOT: org.javamoney:javamoney-parent:pom:1.4-SNAPSHOT was not found

Is there any guide or readme i can follow to setup the project on my machine?

Thx for help. Kr

@keilw keilw added the deferred label Sep 8, 2021
@keilw keilw added this to the .Next milestone Sep 8, 2021
@keilw
Copy link
Member

keilw commented Sep 8, 2021

Thanks for mentioning that, unless related to #370 it still has a slightly lower prio but for any build job I'll try to get the org.javamoney:moneta-parent:1.4.3-SNAPSHOT out to the repositories (only Sonatype now AFAIK) soon.
We also had a holdup due to issues with the API keys for Sonatype deployment, sorry for that, but should be deployable any time soon.

@keilw keilw modified the milestones: 1.5, .Next Aug 25, 2022
@keilw keilw modified the milestones: .Next, 1.5 Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants