1
votes

We have below method in a HeartBeat thread which runs every 30 seconds, we have introduced Guava cache with 5 minutes refresh as below for ClientsDAO.getClients() so that we dont hit the database for every 30 seconds.

private List<String> getClients() {
        final Supplier<List<String>> supplier = () ->  ClientsDAO.getClients();
        if(Server.CACHE_REFRESH_DURATION <=0)
            return supplier.get();
        else{
        LOG.debug("Fetching Clients from cache, duration = "+Server.CACHE_REFRESH_DURATION+". timeunit = "+Server.CACHE_REFRESH_TIMEUNIT);  
        return Suppliers.memoizeWithExpiration(supplier, Server.CACHE_REFRESH_DURATION, Server.CACHE_REFRESH_TIMEUNIT).get();
        }       
    }

As you can see in the below log every time thread HeartBeat runs its hitting the database instead of fetching it from the cache. can someone please help me fix it?

[Jun 10 10:16:05] pool-16-thread-1 | DEBUG | com.server.Heartbeat | Fetching Clients from cache, duration = 5. timeunit = MINUTES
[Jun 10 10:16:05] pool-16-thread-1 | DEBUG | com.server.ClientsDAO | Getting DB connection
[Jun 10 10:16:05] pool-16-thread-1 | DEBUG | com.server.ClientsDAO | Queried for Clients

[Jun 10 10:16:35] pool-16-thread-1 | DEBUG | com.server.Heartbeat | Fetching Clients from cache, duration = 5. timeunit = MINUTES
[Jun 10 10:16:35] pool-16-thread-1 | DEBUG | com.server.ClientsDAO | Getting DB connection
[Jun 10 10:16:35] pool-16-thread-1 | DEBUG | com.server.ClientsDAO | Queried for Clients

[Jun 10 10:17:05] pool-16-thread-1 | DEBUG | com.server.Heartbeat | Fetching Clients from cache, duration = 5. timeunit = MINUTES
[Jun 10 10:17:05] pool-16-thread-1 | DEBUG | com.server.ClientsDAO | Getting DB connection
[Jun 10 10:17:05] pool-16-thread-1 | DEBUG | com.server.ClientsDAO | Queried for Clients
1
Are you really still using Guava 11 ?Marged
@Marged its an old application which we are making some changes after many years, which version do you recommend? 27.1-jre?RanPaul
You're returning new instance of memoizing Supplier each time, aren't you?Xaerxess
Any version that fits your other dependenciesMarged

1 Answers

7
votes

You never reuse Suppliers.memoizeWithExpiration so it's always a new call

You are creating a new memoizing supplier at each call, so you basically make a new call each time because the new memoizing supplier is empty and therefore propagates the call to fill itself. You should create the memoizing supplier only once, and call it repeatedly like this :

private final Supplier<List<Client>> getClientsSupplier = Suppliers.memoizeWithExpiration(ClientsDao::getClients, Server.CACHE_REFRESH_DURATION, Server.CACHE_REFRESH_TIMEUNIT);

public List<Client> getClients() {
  return getClientsSupplier.get();
}