The stats ticker does not use the builder's ticker.

[jwiklund]

Building a cache loader that uses both stats and expiration (eg recordStats and expireAfterWrite) and providing a ticker for testing builds a cache that uses system time for stats and the provided ticker for expiration.

This can become a problem because the CacheStats constructor validates that variables are not negative, which they can become if the stubbed ticker's time is in the future (or more than System.nanoTime), in general it's possible to test refresh by ensuring that start time + refresh time is never more than System.nanoTime, but when the computer is restarted (On my linux desktop the jvm seems to use time since startup) tests can not be made to pass until the uptime is more than the refresh time.

[jwiklund]

NB I expect this to pass:

 class FakeTicker implements Ticker {

    private final AtomicLong nanos = new AtomicLong();

    void advance(Duration duration) {
      nanos.addAndGet(duration.toNanos());
    }

    @Override
    public long read() {
      return nanos.get();
    }
  }

  @Test
  public void test() {
    FakeTicker ticker = new FakeTicker();
    // simulate a low nanoTime by setting the ticker to now.
    ticker.nanos.set(System.nanoTime());

    LoadingCache<Object, String> cache = Caffeine.newBuilder()
        .recordStats()
        .refreshAfterWrite(Duration.ofSeconds(10))
        .ticker(ticker)
        .build((ignoredKey) -> "ok");

    new CacheMetrics(metric_registry).monitor(cache, "country-attributes-cache");

    assertThat(cache.get("1"), is("ok"));

    ticker.advance(Duration.ofSeconds(11));

    assertThat(cache.get("1"), is("ok"));

    cache.stats();
  }

Currently it throws

java.lang.IllegalArgumentException
	at com.github.benmanes.caffeine.cache.stats.CacheStats.<init>(CacheStats.java:109)
	at com.github.benmanes.caffeine.cache.stats.ConcurrentStatsCounter.snapshot(ConcurrentStatsCounter.java:95)
	at com.github.benmanes.caffeine.cache.LocalManualCache.stats(LocalManualCache.java:89)

[ben-manes]

Thanks! I see that the cause is due refreshIfNeeded using the expirationTicker() for the initial timestamp, and the statsTicker() when to calculate the load time. For all other calls, the timestamps are independent and, usually, the reuse is not harmful since they are both System.nanoTime(). There is therefore two options for us to consider,

1. Make an additional call to statsTicker().read() prior to refreshing the value. This would make the timestamps independent as expected elsewhere.
2. Make the statsTicker configurable by using the supplied ticker. These would stay separate internally so that turning on expiration doesn't penalize with statistics timestamp reads, or vice versa. However, manipulating time in a test to go backwards would result in this same exception.

Guava does not use the configured ticker in its stats recording and I don't see a good reason to for users to configure it. So my preference would be (1). Any objections?

Reviewing the usages, all others are correctly using the statsTicker for the start time. One bug does appear to be in AsyncBulkCompleter which accidentally uses recordLoadSuccess(result.size()) instead of recordLoadSuccess(loadTime). So that should be fixed as well.

[ben-manes]

There's a few items on the backlog, but I'll try to wrap them up and release this weekend.

[ben-manes]

Released 2.7