Millisecond precision for expiration

[ghost]

Would it be possible to relax this condition and check that entries are expired after at least one second?

https://github.com/jsr107/jsr107tck/blob/master/cache-tests/src/test/java/org/jsr107/tck/expiry/CacheExpiryTest.java#L155

[gregrluck]

Is that the only place you want a larger delay?

[ghost]

Hi Greg. Yes, this is the only place were we need a larger delay.

[chrisdennis]

Shouldn't an implementation always round TTI/TTL times down so that this change becomes unnecessary? If a user asks for 20ms TTL that should be always rounded down to 0 seconds (i.e. immediate expiry) rather than rounding up to 1 second.

[chrisdennis]

Okay, now I'm confused. To the best of my knowledge this test wasn't buggy before this change, and now it's coverage is weaker, and it takes longer to run. If an implementation can't pass a test, and nobody can give an implementation independent explanation for how the test is broken (i.e. asserting something beyond or outside what the spec guarantees) then surely the bug must lie with implementation and not the TCK?

[gregrluck]

The spec says that the minimum time unit for Duration is milliseconds. i.e. from the Expiry section "The minimum allowed TimeUnit is TimeUnit.MILLISECONDS."

Windows XP's System.currentTimeMillis is accurate to +- 15ms. See http://stackoverflow.com/questions/7859019/system-currenttimemillis-is-not-accurate-on-windows-xp. So, the original test set an expiry of 20ms but then waited 50ms.

We are changing this one test to be more permissive. Some platforms/implementations may not be able to get down to the millisecond level. The spec says that milliseconds is the minimum time unit - it does not say that an implementation must expire in these short intervals.

Brian and I chatted about this. We think it is Ok to be more permissive here. Yes it does the extend the running time of the tests.

[chrisdennis]

I know i'm probably flogging a dead horse here, but to me this is an important horse, and as Monty Python would say: it's not dead yet.

If the problem we're worried about here is that the platform on which the test is running might not allow for the detection of the passage of such a small amount of time then why don't we fix the test to validate that the platform did indeed see that amount of time elapse. Then since we know that the implementation under test has access to the same information, we can assert that it should have expired the entry.

As it stands expanding the sleep time all the way out to 1100ms opens up the potential for implementations to violate data freshness guarantees that the spec guarantees to the user. This allows implementations to return stale data. Specifically (as I hinted at in my first comment) an implementation is now free to round TTIs/TTLs up to the nearest second and still pass this test. Rounding up a TTI/TTL is not a good thing, TTI/TTLs should always be rounded down, so that worst case things are expired early and no stale data is returned (modulo the limitations of the platform which obviously cannot be avoided).

[cruftex]

@chrisdennis: From that perspective: it was dead when born.

This allows implementations to return stale data.

What is stale? The specification is not precise enough for that to decide.

Example 1: In a read through configuration the loader takes 5 minutes and the expiry policy returns that the value should expire in 2 minutes. Does this mean the data is allowed to be 7 minutes old?

Example 2: What happens under high load and the thread returning the duration is scheduled away for 345ms? What is the time reference of the returned duration?

1. The current spec does not allow precise expiry and is rather weak. I would think, that the expiry rules in the spec are more targeted towards resource control.
2. If an application needs to have precise control of the life time and is not allowed "to return stale data" there needs to be additional time checks inside the application.
3. I think having precise expiry (or sharp expiry) is something desirable from a cache. I would second a proposal for that.

[chrisdennis]

Oh, believe me I understand that there may be issues in the specification around what should or should not be considered expired and when the life of a cache entry is consider to have started. Those are questions we should ask of the specification however, they do not need to be discussed here - this is a TCK question not a spec one.

As regards whether the expiry is intended to be used as resource control, or for data freshness I can tell you that Ehcache's take on this is (and has been for many years) that it is exclusively for data freshness, and that capacity eviction is how you control resources. Anything else is a misuse and a fairly dangerous one at that.

Right now I'm concerned with the fact that we've loosened the TCK so that an implementation can pass it. @gregrluck makes a valid point that the test as it stood could be problematic on some platforms, however I think if that was the motivation behind this change then what was done to fix it was overkill. I think the following for example is a fair test:

Cache cache = createCacheWithTTL(20, TimeUnit.MILLISECONDS);
cache.put("key", "value");
long timeDefinitivelyAfterPutHasCompleted = System.currentTimeMillis();
while (System.currentTimeMillis() < (timeDefinitivelyAfterPutHasCompleted + 21)); //leaving aside fencepost issues
assertThat(cache.get("key"), nullValue());

Would others agree that the above is a fair test? Even if we disagree on whether or not people use expiry for resource control, do we agree that if any users use it for enforcing data freshness that allowing implementations to round up fundamentally breaks those usecases.

[cruftex]

+1