mybatis / mybatis-3

MyBatis SQL mapper framework for Java
http://mybatis.github.io/mybatis-3/
Apache License 2.0
19.71k stars 12.82k forks source link

Enable access to Cache Delegates #572

Open FlorianSchaetz opened 8 years ago

FlorianSchaetz commented 8 years ago

When using a Cache, especially when using custom Cache implementations, it can be desirable to access that Cache object. Unfortunately, most of the time it will be wrapped into a LoggingCache or similar, which hides this implementation. If would be helpful if LoggingCache, etc. offered a method to get their delegate, so that access to the real Cache instances would be possible. While it is true that direct access to an underlying object might break behaviour of the Cache chain, without such an access, custom Caches will have to be registered into a custom HashMap or similar which tends also be error-prone, especially in a concurrent environment.

terba commented 8 years ago

I would also use this feature on the default cache implementation to access the LoggingCache decorator to get its statistical data. Currently the only accessible statistical value of a cache is its size. Hit ratio is only available in the logs, but I would like to publish that info too on our app's API.

If you are there, please create also public getters for hits, requests and hitRatio attributes as well. Thanks in advance!

wuwen5 commented 7 years ago

While it is true that direct access to an underlying object might break behaviour of the Cache chain, without such an access, custom Caches will have to be registered into a custom HashMap or similar which tends also be error-prone, especially in a concurrent environment.

If custom cache can extends from default cache(PerpetualCache), retain the default cache decorators, this is also possible.

like this https://github.com/mybatis/mybatis-3/pull/818 @harawata Do you agree with this process?if ok, I reopen this

harawata commented 7 years ago

@wuwen5 Hi, Requiring a custom cache to extend PerpetualCache seems to be a big constraint. I have just started looking into cache related issues, so please give me some time to figure out a way to achieve what you guys want.

harawata commented 7 years ago

@FlorianSchaetz ,

You wrote...

most of the time it will be wrapped into a LoggingCache or similar

What did you mean by "or similar" here? I can see a custom cache is wrapped by LoggingCache, but don't see 'similar' cases.

@terba @wuwen5 ,

To log statistics, isn't it better to use the statistics provided by each custom cache implementation like below?

If so, just stop wrapping custom caches in LoggingCache could be a solution.

p.s. By 'custom caches', I meant the external 3rd party cache implementations and not the internal ones.

terba commented 7 years ago

@harawata: I would use that feature with the default (myBatis internal) cache implementation.

harawata commented 7 years ago

@terba Yes, you wrote that in your previous comment. Sorry I had missed it!

I am wondering if it's possible to satisfy @FlorianSchaetz 's request (i.e. accessing external custom cache implementation) without exposing cache delegates. It wouldn't solve your issue, but one problem at a time. :)

wuwen5 commented 7 years ago

Hi @harawata , I have an idea, we can create an abstract cache decorator class. like this #838, Can you agree this way?

@Test
  public void getSpecifiedDecorator() {
    Cache cache = new PerpetualCache("default");
    cache = new LruCache(cache);
    cache = new LoggingCache(cache);
    cache = new SynchronizedCache(cache);

    cache.putObject("hello", System.currentTimeMillis());
    cache.getObject("hello");

    LoggingCache loggingCache = findCacheDecorator((CacheDecorator)cache, LoggingCache.class);

    assertNotNull(loggingCache);
    assertEquals(1,loggingCache.getHits());

    PerpetualCache cacheDecorator = findCacheDecorator((CacheDecorator) cache, PerpetualCache.class);

    assertNotNull(cacheDecorator);
  }

  private <T> T findCacheDecorator(CacheDecorator cache, Class<T> type) {
    Cache delegate = cache.getDelegate();
    if (delegate.getClass().equals(type)) {
      return (T) delegate;
    } else if (delegate instanceof  CacheDecorator) {
      return findCacheDecorator((CacheDecorator)delegate, type);
    }
    return null;
  }
wuwen5 commented 7 years ago

and this eliminates duplicate code for the cache decorator implements.

harawata commented 7 years ago

Hi @wuwen5 ,

Thank you for the PR, but I don't think allowing access to the delegates is a good idea. For example, SynchronizedCache is useless if your code accesses the underlying delegate. That is why I am trying to solve this issue ( @FlorianSchaetz 's issue) in a different way.

Regarding @terba 's request, it may be useful to provide basic statistics like hit ratio when using the built-in caches, but providing it via LoggingCache does not seem to be right approach, in my opinion.

daitangio commented 1 year ago

I have similar trouble: I need to get some more stats like max cache size during its life, and I cannot just extend PersistentCache. there is a way to provide a Custom decorator to the CacheBuilder or to extend it a bit? Also why it is not possible to apply decrators on top of custom caches? I mean...this code inside Cahce Builder

  public Cache build() {
    setDefaultImplementations();
    Cache cache = newBaseCacheInstance(implementation, id);
    setCacheProperties(cache);
    // issue #352, do not apply decorators to custom caches
    if (PerpetualCache.class.equals(cache.getClass())) {
      for (Class<? extends Cache> decorator : decorators) {
        cache = newCacheDecoratorInstance(decorator, cache);
        setCacheProperties(cache);
      }
      cache = setStandardDecorators(cache);
    } else if (!LoggingCache.class.isAssignableFrom(cache.getClass())) {
      cache = new LoggingCache(cache);
    }
    return cache;
  }

cui prodest? why we avoid decorators for custom caches, but we support LoggingCache?

harawata commented 1 year ago

@daitangio Could you elaborate on your requirements?

daitangio commented 1 year ago

For sure. I need a way to access to all the caches to collect stats about their usage (for instance how much % is used and so on). I need to do maths on these stats.

The only way I find out was to define a custom cache: but doing so I loose all the standard decorators (like LRU + ability to flush them and so on) and it was a disaster :) See "// issue #352, do not apply decorators to custom caches" above.

It could be nice to extend the LoggingCache class if possible or to be able to decorate a custom perpetual cache... OR there is a simple way to access all the PerpetualCache's instances? Thank you for your prompt response!

harawata commented 1 year ago

Thank you for a reply, @daitangio !

I am thinking about adding unwrap(Class) method to org.apache.ibatis.cache.Cache (like JCache's Cache interface). Basically, by calling cache.unwrap(PerpetualCache.class), you can get the PerpetualCache instance. Does this solve your problem? (I would appreciate inputs from other commenters as well!)

As it involves other modification and will be a backward incompatible change, you may have to wait for the next major (minor?) version which is under discussion currently.

p.s. I know this contradicts my earlier comments about allowing access to the delegate and I still have concerns, but it could be fine if it's used properly...

daitangio commented 1 year ago

Hi, I only need two things:

  1. a static method to access all the caches like List getAllActiveCaches() because the cache interface already expose getSize().

  2. a way to understand the current cache capacity, like float getCapacity() with a value between 0 and 1 (=100% capacity).

The (2) is not mandatory because you can "discover" the underlying implementation easily, but the first one is needed and as far as I know, there is no way to obtain it (but I may be wrong).