Listener behavior on 'replace' with an immediately expiring value.

[chrisdennis]

While attempting to cleanup the Ehcache 3 behavior around 'immediately expired' values I ran in to some issues that I was hoping others might be able to clarify for me. If I were to execute the following class should a JSR-107 compliant cache print?

import javax.cache.*;
import javax.cache.configuration.*;
import javax.cache.event.*;
import javax.cache.expiry.*;

public class ListenerExample {

  public static void main(String[] args) {
    CacheManager cm = Caching.getCachingProvider().getCacheManager();
    try {
      MutableConfiguration<String, String> config = new MutableConfiguration<String, String>();
      config.setExpiryPolicyFactory(ImmediateOnUpdateExpiryPolicy.createFactory());
      config.addCacheEntryListenerConfiguration(LoggingEventListener.<String, String>createConfig());

      Cache<String, String> cache = cm.createCache("cache", config);
      System.out.println("value is : " + cache.get("key"));
      cache.put("key", "1");
      System.out.println("value is : " + cache.get("key"));
      cache.replace("key", "2");
      System.out.println("value is : " + cache.get("key"));
    } finally {
      cm.close();
    }
  }

  static class ImmediateOnUpdateExpiryPolicy implements ExpiryPolicy {

    private static Factory<? extends ExpiryPolicy> createFactory() {
      return new Factory<ExpiryPolicy>() {
        @Override
        public ExpiryPolicy create() {
          return new ImmediateOnUpdateExpiryPolicy();
        }
      };
    }

    @Override
    public Duration getExpiryForCreation() {
      return Duration.ETERNAL;
    }

    @Override
    public Duration getExpiryForAccess() {
      return null;
    }

    @Override
    public Duration getExpiryForUpdate() {
      return Duration.ZERO;
    }
  }

  static class LoggingEventListener implements 
          CacheEntryCreatedListener<Object, Object>, CacheEntryUpdatedListener<Object, Object>,
          CacheEntryRemovedListener<Object, Object>, CacheEntryExpiredListener<Object, Object> {

    private static <K, V> CacheEntryListenerConfiguration<K, V> createConfig() {
      return new CacheEntryListenerConfiguration<K, V>() {
        @Override
        public Factory<CacheEntryListener<? super K, ? super V>> getCacheEntryListenerFactory() {
          return new Factory<CacheEntryListener<? super K, ? super V>>() {
            @Override
            public CacheEntryListener<? super K, ? super V> create() {
              return new LoggingEventListener();
            }
          };
        }

        @Override
        public boolean isOldValueRequired() {
          return true;
        }

        @Override
        public Factory<CacheEntryEventFilter<? super K, ? super V>> getCacheEntryEventFilterFactory() {
          return null;
        }

        @Override
        public boolean isSynchronous() {
          return true;
        }
      };
    }

    @Override
    public void onCreated(Iterable<CacheEntryEvent<? extends Object, ? extends Object>> events) throws CacheEntryListenerException {
      for (CacheEntryEvent<?, ?> event : events) {
        System.out.println("created : " + event.getKey() + ":" + event.getValue() + " was " + event.getOldValue());
      }
    }

    @Override
    public void onUpdated(Iterable<CacheEntryEvent<? extends Object, ? extends Object>> events) throws CacheEntryListenerException {
      for (CacheEntryEvent<?, ?> event : events) {
        System.out.println("updated : " + event.getKey() + ":" + event.getValue() + " was " + event.getOldValue());
      }
    }

    @Override
    public void onRemoved(Iterable<CacheEntryEvent<? extends Object, ? extends Object>> events) throws CacheEntryListenerException {
      for (CacheEntryEvent<?, ?> event : events) {
        System.out.println("removed : " + event.getKey() + ":" + event.getValue() + " was " + event.getOldValue());
      }
    }

    @Override
    public void onExpired(Iterable<CacheEntryEvent<? extends Object, ? extends Object>> events) throws CacheEntryListenerException {
      for (CacheEntryEvent<?, ?> event : events) {
        System.out.println("expired : " + event.getKey() + ":" + event.getValue() + "was " + event.getOldValue());
      }
    }
  }
}

I think I know what the spec says it should print, and I know what I would like it to print - and they are not the same things. I'd like to get other peoples take on this before I figure out if we have a bug worth raising here. (Note: it seems the TCK doesn't seem to have a position on this at the moment).

[cruftex]

Hi Dennis, just wondering: What about putting it into a JUnit test case and run that against all current implementations? This is the reason why I did the "test-zoo" project: https://github.com/cruftex/jsr107-test-zoo

[chrisdennis]

Right, I'm really not interested in knowing what the various implementations do. I obviously know what Ehcache does, and I know what the TCK enforces, and therefore I know what possibilties that leaves open for the other implementors. I'm more interested in what people think it should do, and what people think the specification says it should do (and whether those two are the same thing).

[cruftex]

I would see an existing implementation as condensed thoughts of many people :)

[cruftex]

My (quick) thought: The replaced value does never make it into the cache and the events should reflect the effective changes on the cache state. So, according to that (theory) the last two lines could be:

expired : key: null was 1 value is : null

Disclaimer: In my JSR107 implementation (cache2k) I am done with everything except events...

[cruftex]

Meanwhile I implemented the events myself and know the TCK tests on it. I think there are two possible variants.

Variant 1:

• updated : key: 2 was 1
• expired : key: null was 2
• value is : null

Variant 2:

• expired : key: null was 1
• value is : null

@chrisdennis: Is this what you had in mind?

[chrisdennis]

Yes this is what I had in mind, I consider the first variant to be 'correct' for various reasons:

• consistency with the replace return
• not expiring a value that was judged "eternal" on addition
• allows a listener to keep a secondary data structure in sync with the cache

Whats confusing is that I believe I filed the issue because there was language in the spec that indicated (strongly) that the first variant was the expected one... but that the TCK didn't cover this case. Looking now however I can't find this language anywhere in the spec... so maybe I imagined the whole thing. If nobody can find or recall that language i was talking about then we can close this whole issue as the product of my fevered imagination.

[cruftex]

My thinking is:

Pro Variant 1: The listeners should know what happened in the cache

Pro Variant 2: The expectation of synchronous listeners is that they replicate the cache state changes, not the actions on it. The value 2 never becomes visible. Sending the update event would pretend so.

I think variant 2 is the more consistent one.

Weirdly, if you do a put instead of replace, the cache would not send an update event, but invoke the writer.

It's good to discuss about those corner cases, to see whether we have gaps in the Spec and TCK. OTOH, if someone is doing this kind of configuration, the probability is quite high that any behavior will not meet the idea of the user...

[gregrluck]

Interesting corner case. The more general case here is what are the effects of putting into a cache an immediately expired entry.

My thoughts are expiry is viewed from the perspective from those outside the cache requesting data.

So a put or update of an immediately expired entry should generate the following events:

put - created and expired update - update and expired

It should also call through to the integration package. So CacheWriter should be called.

For statistics: put - put count is incremented update - put count is incremented

According to Jens, there are TCK tests that do not follow the above behaviour.

Proposal is to clarify the above behaviour in the spec and in the TCK.

I think the behaviour is useful if you had your cache integrated to other systems. You are using the write-through pattern and want the other systems to get the puts. e.g. a write to a database or a listener connected to another system. However you know this cache will never get a request for the data so you don't want to keep it around. So you mark it as immediately expiring.

[gregrluck]

Jens has proposed that this is such a corner case that maybe we do nothing on this for 1.1