Cache 2 CacheLoader is called on every cache read.

[Bernard Jaquet]

Hello everyone

I've been checking out the new caching service of JIRA (cache 2) and discovered a strange fact while debugging my code: My CacheLoader implementation is called on every access of the cache (which makes the caching useless) and I've been wondering if I do something wrong. Here is my Code for the cache call:

CacheSettingsBuilder cacheSettingsBuilder = new CacheSettingsBuilder();
velocities = cacheManager.getCache("VelocityCache", new VelocityCacheLoader(),
        cacheSettingsBuilder.expireAfterWrite(1, TimeUnit.DAYS).build())
        .get(Integer.valueOf(this.rapidBoard));

And here is the CacheLoaderImplementation:

public class VelocityCacheLoader implements CacheLoader<Integer, VelocitiesBean> {
    private final Logger log = LoggerFactory.getLogger(VelocityCacheLoader.class);
    private final OperatingFiguresUtils operatingFiguresUtils = new OperatingFiguresUtils();

    @Override
    public VelocitiesBean load(Integer rapidBoardId) {
        try {
            List<SprintBean> sprints = operatingFiguresUtils.getSprints(rapidBoardId);
            VelocitiesBean velocities = new VelocitiesBean();
            if (sprints.size() == 0) {
                velocities.setLastSprintVelocity(0d);
                velocities.setShortTermVelocity(0d);
                velocities.setLongTermVelocity(0d);
                return velocities;
            }
            SprintBean lastSprint = sprints.get(0);
            sprints.remove(0);
            List<SprintBean> lastSprintList = new ArrayList<SprintBean>();
            lastSprintList.add(lastSprint);
            List<SprintBean> shortTermSprints = operatingFiguresUtils.getSprintsFromInterval(sprints,
                    lastSprint.getStartDate(), OperatingFiguresUtils.SHORT_TERM_EVALUATION_INTERVAL);
            List<SprintBean> longTermSprints = operatingFiguresUtils.getSprintsFromInterval(sprints,
                    lastSprint.getStartDate(), OperatingFiguresUtils.LONG_TERM_EVALUATION_INTERVAL);

            velocities.setLastSprintVelocity(calculateSprintsVelocity(lastSprintList));
            velocities.setShortTermVelocity(calculateSprintsVelocity(shortTermSprints));
            velocities.setLongTermVelocity(calculateSprintsVelocity(longTermSprints));
            return velocities;
        } catch (Exception e) {
            log.error("Calculation of velocity failed because: " + e.getMessage(), e);
            throw new RuntimeException(e);
        }
    }
}

I am working on JIRA 6.2.7. Anyone any Idea what this could be?

Best regards

Bernard

[crf · Accepted solution]

What's wrong is that you are constantly throwing away your cache and creating a new one.  The JavaDocs are not very clear about this.  You should create the cache once in your constructor and reuse it, not call getCache again every time you want to use it.

Example of correct pattern:

private final Cache<Long, Foo> cacheById;
private final FooDao fooDao;
 
public FooManager(CacheManager cacheManager, FooDao fooDao)
{
    this.fooDao = fooDao;
    this.cacheById = cacheManager.getCache(getClass().getName() + ".cacheById",
            new ByIdCacheLoader(),
            new CacheSettingsBuilder().expireAfterAccess(1, TimeUnit.DAYS).build());
}
 
...
 
@Override
public Foo findById(Long id)
{
    // Note: Assuming that "Foo" objects are immutable.
    // Otherwise, you should create a copy so the cached object cannot
    // be altered by the caller.
    return cacheById.get(id);
}
 
class ByIdCacheLoader implements CacheLoader<Long, Foo>
{
    @Override
    public Foo load(final Long id)
    {
        return fooDao.findById(id);
    }
}

[crf]

You may be wondering how that works if two different classes want to share the same cache. The answer is that it doesn't, because you should never, ever do such a thing in the first place. A cache needs to have one clear owner to ensure that it is interacted with correctly and that its consistency is properly maintained. If multiple classes mess around with it, then this violates the https://en.wikipedia.org/wiki/Law_of_Demeter -- Too many things know too much about it. The proper answer is to create a new component that is the sole owner of the cache and make the classes that were going to talk to the cache directly talk to that component instead.

[Bernard Jaquet]

I assume, that in case of a WebAction, the cache object must be kept for all calls, which basically means making it static.

[crf]

While that is possible, it is not what I would recommend. I would instead recommend that you make a dedicated component that holds the cache and let that component be injected into your action. This has the advantage of being something that you can more easily unit test.