Uploaded image for project: 'Infinispan'
  1. Infinispan
  2. ISPN-10171

Spring Cache sync locking does not work

    Details

    • Workaround Description:
      Hide

      I have worked around the problem by overriding the SpringEmbeddedCacheManager as follows:

          @Bean
          public SpringEmbeddedCacheManager springEmbeddedCacheManager(final EmbeddedCacheManager embeddedCacheManager) {
      
              return new SpringEmbeddedCacheManager(embeddedCacheManager) {
      
                  private final Map<String, SpringCache> cachesByName = new ConcurrentHashMap<>();
      
                  @Override
                  public SpringCache getCache(final String name) {
                      return cachesByName.computeIfAbsent(name, super::getCache);
                  }
              };
          }
      
      Show
      I have worked around the problem by overriding the SpringEmbeddedCacheManager as follows: @Bean public SpringEmbeddedCacheManager springEmbeddedCacheManager( final EmbeddedCacheManager embeddedCacheManager) { return new SpringEmbeddedCacheManager(embeddedCacheManager) { private final Map< String , SpringCache> cachesByName = new ConcurrentHashMap<>(); @Override public SpringCache getCache( final String name) { return cachesByName.computeIfAbsent(name, super ::getCache); } }; }

      Description

      With ISPN-7224/https://github.com/infinispan/infinispan/pull/5262, an implementation was written to support a synchronous get that implements Spring Cache's @Cacheable(sync = true).

      That seems to work well in isolation, but not in an integrated way. By that I mean, that the implementation of SpringCache maintains its own {{ReentrantLock}}s in each instance.

      Zooming out a bit, this is part of the call stack when the SpringCache instance is looked up during handling the @Cacheable(sync = true) annotation:

      CacheInterceptor.invoke(MethodInvocation)  (org.springframework.cache.interceptor)
        CacheAspectSupport.execute(CacheOperationInvoker, Object, Method, Object[])  (org.springframework.cache.interceptor)
          CacheOperationContexts in CacheAspectSupport.CacheOperationContexts(Collection<? extends CacheOperation>, Method, Object[], Object, Class<?>)  (org.springframework.cache.interceptor)
            CacheAspectSupport.getOperationContext(CacheOperation, Method, Object[], Object, Class<?>)  (org.springframework.cache.interceptor)
              CacheOperationContext in CacheAspectSupport.CacheOperationContext(CacheOperationMetadata, Object[], Object)  (org.springframework.cache.interceptor)
                CacheAspectSupport.getCaches(CacheOperationInvocationContext<CacheOperation>, CacheResolver)  (org.springframework.cache.interceptor)
                  AbstractCacheResolver.resolveCaches(CacheOperationInvocationContext<?>)  (org.springframework.cache.interceptor)
         ----->>    SpringEmbeddedCacheManager.getCache(String)  (org.infinispan.spring.embedded.provider)
                      SpringCache.SpringCache(BasicCache)  (org.infinispan.spring.common.provider)
                        SpringCache.SpringCache(BasicCache, long, long) (org.infinispan.spring.common.provider)
      

      The problem with this is, that for each turn the interceptor handles the request a new instance of the SpringCache is made in the SpringEmbeddedCacheManager, rendering the use of the ReentrantLock mechanism useless to a certain extent, because multiple threads accessing the @Cacheable(sync = true) method will each have their own SpringCache and corresponding ReentrantLock.

      In turn, there is no locking at all, and each thread will be executing the (assumed to be) expensive code that is intended to be cached.

      I'm intending to write a patch for this, but this would be the first time I commit to an open source project (bear with me ). I'll try to provide a pull request for this.

        Gliffy Diagrams

          Attachments

            Activity

              People

              • Assignee:
                karesti Katia Aresti
                Reporter:
                rensvanleeuwen Rens van Leeuwen
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: