SOLR-15883 Fix a problem in OpenExchangeRatesOrgProvider: introduce a parameter so that it downloads currency rates while indexing not while searching

[arobinski]

https://issues.apache.org/jira/browse/SOLR-15883

Description

Currently the currency fields (specifically: fields of type solr.CurrencyFieldType that use OpenExchangeRatesOrgProvider) refresh the currency rates during a search request. If the openexchangerates.org service is unavailable or slow, all search requests (that use currencies) may fail.

This MR introduces a new boolean parameter refreshWhileSearching to fields of type solr.CurrencyFieldType that have providerClass equal to solr.OpenExchangeRatesOrgProvider. If the parameter is true (default), it will work as before. If it's false, then currency rates will NOT be refreshed during a search request.

Additionally, the MR introduces a new class: OpenExchangeRatesOrgReloader, which can listen to newSearcher and firstSearcher events and reload currency rates when a new searcher is opened. If you want to use the OpenExchangeRatesOrgReloader, it has to be added to solrconfig.xml.

This is my first MR to the SOLR project and I am not sure if I did everything properly. I'd be grateful for a review from someone who knows the SOLR code better.

Solution

Instead of refreshing currency rates during a search request, refresh them during commit. More information above.

Tests

I modified one test and added one test to the OpenExchangeRatesOrgProviderTest class.

I have run ./gradlew check and the test ChaosMonkeyNothingIsSafeWithPullReplicasTest failed, but it seems not related to my changes.

Checklist

Please review the following and check all that apply:

• [x] I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• [x] I have created a Jira issue and added the issue ID to my pull request title.
• [x] I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
• [x] I have developed this patch against the main branch.
• [x] I have run ./gradlew check.
• [x] I have added tests for my changes.
• [ ] I have added documentation for the Reference Guide

[janhoy]

Interesting solution to this problem. Another way to fix it would be for the CurrencyFieldType to register ScheduledExecutorService and do the reloading in a tread-safe manner in that thread. It would be more automatic without new params or classes, and it would respect the reloadInterval setting as before, only it would do the reload in the executor instead of in the query thread. I think it is quite resource efficient as well, you won't occupy a thread except for each run of the executor. WDYT?

[arobinski]

This seems like a much better solution than mine. I will try to implement it in this pull request.

[janhoy]

This seems like a much better solution than mine. I will try to implement it in this pull request.

I'd look at creating a pool ScheduledExecutorService.newMDCAwareSingleThreadExecutor(new SolrNamedThreadFactory("currencyProvider")), and in reload() wrap the logic inside a Runnable that is passed to the exector and then return immediately.

That way, we only refresh rates if there are actual search requests, but once a request comes, it will only trigger a background refresh, while the request will succeed on the existing rates (stale).

If you add proper synchronization on read/write of the protected OpenExchangeRates rates class, then the amount of time you need to lock while swapping in the new object should be minimal.

[arobinski]

@janhoy I tried doing as you wrote. I created an ExecutorService using ExecutorUtil.newMDCAwareSingleThreadExecutor . It's on line 70 in OpenExchangeRatesOrgProvider.java. It generally seems to work, but the problem is that SOLR does not terminate properly now. The reason is that I never call executorService.shutdown(). I don't know from where I can call the shutdown method. Maybe there is a SOLR shutdown event that I can subscribe to? Could you give me a hint, please?

[arobinski]

I'm closing the pull request. I don't know how to properly develop the fix and I didn't receive enough guidance.

[janhoy]

Self assigning and reopening for another iteration.

[github-actions[bot]]

This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution!