RFR: 1052: Memory leak in RestRequestCache

[Erik Joelsson]

On Wed, 26 May 2021 07:40:59 GMT, Erik Helin <ehelin at openjdk.org> wrote:

>> bot/src/main/java/org/openjdk/skara/bot/BotRunnerConfiguration.java line 328:
>> 
>>> 326:     Duration cacheEvictionInterval() {
>>> 327:         if (!config.contains("runner") || !config.get("runner").contains("cache_eviction_interval")) {
>>> 328:             var defaultValue = Duration.ofMinutes(5);
>> 
>> Have you done any testing to see if a default interval of 5 minutes is the right frequency for cache evictions? Naively, it seems like it might be too often. Is there a performance impact?
>
> I agree with Kevin, I _think_ we could probably keep at least 60 minutes of cache, if not even more. Or I might have missed something ����

That is a good point. In my initial patch I used the scheduling frequency (which is what the watchdog uses) instead of this new configuration value, which is 10 seconds, and it worked fine on staging. I thought that was a bit excessive so I increased to a default of 5 mins, which matches the lowest "maxAge" in the cache.

While we could certainly live with a longer interval here, 24h would still solve our problems, I don't think we have much to gain from increasing it longer than my suggested 5 mins. The nature of the bots is that they run pretty much non stop, using a lot of cpu. I can't see how running this little method will have any noticeable impact on performance, even if the cache has thousands or even millions of valid entries. It shouldn't take more than milliseconds on one of 8-32 threads (depending on bot runner config).

If we start seeing problems, this value can always be changed in the future, or configured for any specific bot runner configuration.

-------------

PR: https://git.openjdk.java.net/skara/pull/1167