sixhours-team / memcached-spring-boot

Library that provides support for auto-configuration of Memcached cache in a Spring Boot application.
Apache License 2.0
71 stars 33 forks source link

Use with mcrouter? #92

Closed davidkarlsen closed 4 years ago

davidkarlsen commented 4 years ago

The old version used to work with mcrouter, there seems to be some snag, I get:

Handling exception io.sixhours.memcached.cache.MemcachedOperationException: Fa
iled to touch key
io.sixhours.memcached.cache.MemcachedOperationException: Failed to touch key
        at io.sixhours.memcached.cache.XMemcachedClient.touch(XMemcachedClient.java:74)
        at io.sixhours.memcached.cache.MemcachedCache.put(MemcachedCache.java:109)
        at org.springframework.cache.interceptor.AbstractCacheInvoker.doPut(AbstractCacheInvoker.java:87)
        at org.springframework.cache.interceptor.CacheAspectSupport$CachePutRequest.apply(CacheAspectSupport.java:820)
        at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:429)
        at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:345)
        at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:61)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:747)
        at org.springframework.aop.interceptor.CustomizableTraceInterceptor.invokeUnderTrace(CustomizableTraceInterceptor.java:256)
        at org.springframework.aop.interceptor.AbstractTraceInterceptor.invoke(AbstractTraceInterceptor.java:131)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:747)
        at org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint.proceed(MethodInvocationProceedingJoinPoint.java:88)
        at io.github.resilience4j.circuitbreaker.CircuitBreaker.lambda$decorateCheckedSupplier$82a9021a$1(CircuitBreaker.java:73)
        at io.github.resilience4j.circuitbreaker.CircuitBreaker.executeCheckedSupplier(CircuitBreaker.java:795)
        at io.github.resilience4j.circuitbreaker.configure.CircuitBreakerAspect.defaultHandling(CircuitBreakerAspect.java:183)
        at io.github.resilience4j.circuitbreaker.configure.CircuitBreakerAspect.proceed(CircuitBreakerAspect.java:130)
        at io.github.resilience4j.circuitbreaker.configure.CircuitBreakerAspect.circuitBreakerAroundAdvice(CircuitBreakerAspect.java:107)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:567)
        at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:644)
        at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:633)
        at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:70)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:175)
        at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:747)
        at org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint.proceed(MethodInvocationProceedingJoinPoint.java:88)
        at io.github.resilience4j.retry.Retry.lambda$decorateCheckedSupplier$3f69f149$1(Retry.java:137)
        at io.github.resilience4j.retry.Retry.executeCheckedSupplier(Retry.java:419)
        at io.github.resilience4j.retry.configure.RetryAspect.handleDefaultJoinPoint(RetryAspect.java:176)
        at io.github.resilience4j.retry.configure.RetryAspect.proceed(RetryAspect.java:133)
        at io.github.resilience4j.retry.configure.RetryAspect.retryAroundAdvice(RetryAspect.java:112)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:567)
        at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:644)
        at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:633)
        at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:70)
.....
Caused by: net.rubyeye.xmemcached.exception.MemcachedServerException: unexpected result mc_res_unknown (0) for touch,key=jfr-jfr-srv-d1:corews.accountListCache:namespace
        at net.rubyeye.xmemcached.command.Command.decodeError(Command.java:259)
        at net.rubyeye.xmemcached.command.Command.decodeError(Command.java:274)
        at net.rubyeye.xmemcached.command.text.TextTouchCommand.decode(TextTouchCommand.java:76)
        at net.rubyeye.xmemcached.codec.MemcachedDecoder.decode0(MemcachedDecoder.java:55)
        at net.rubyeye.xmemcached.codec.MemcachedDecoder.decode(MemcachedDecoder.java:50)
        at com.google.code.yanf4j.nio.impl.NioTCPSession.decode(NioTCPSession.java:281)
        at com.google.code.yanf4j.nio.impl.NioTCPSession.decodeAndDispatch(NioTCPSession.java:223)
        at com.google.code.yanf4j.nio.impl.NioTCPSession.readFromBuffer(NioTCPSession.java:194)
        at com.google.code.yanf4j.nio.impl.AbstractNioSession.onRead(AbstractNioSession.java:184)
        at com.google.code.yanf4j.nio.impl.AbstractNioSession.onEvent(AbstractNioSession.java:324)
        at com.google.code.yanf4j.nio.impl.SocketChannelController.dispatchReadEvent(SocketChannelController.java:54)
        at com.google.code.yanf4j.nio.impl.NioController.onRead(NioController.java:150)
        at com.google.code.yanf4j.nio.impl.Reactor.dispatchEvent(Reactor.java:310)
        at com.google.code.yanf4j.nio.impl.Reactor.run(Reactor.java:177)

My mcrouter-config is:

data:
  config.json: |-
    {
      "pools": {
        "A": {
          "servers": [
            // hosts of replicated pool, https://github.com/facebook/mcrouter/wiki/Replicated-pools-setup e.g.:
            "memcached-s1-0.memcached-s1.memcached-s1.svc.icp-global.local:11211",
            "memcached-s1-1.memcached-s1.memcached-s1.svc.icp-global.local:11211",
          ]
        }
      },
      "route": {
        "type": "OperationSelectorRoute",
        "operation_policies": {
          "add": "AllSyncRoute|Pool|A",
          "delete": "AllSyncRoute|Pool|A",
          "get": "LatestRoute|Pool|A",
          "set": "AllSyncRoute|Pool|A"
        }
      }
    }

it works fine if I point directly at the memcached-servers. I guess this is more of a question to the underlying memcached driver implementation - but thought I'd leave a mark here as well.

Do you have any experience using it with mcrouter?

igorbolic commented 4 years ago

Thanks for reporting the issue. Yes, it seems to be Xmemcached related. Unfortunately, haven't used it with mcrouter. I will take a closer look if there is something we can do in the library to remedy the issue.

igorbolic commented 4 years ago

@davidkarlsen Until this issue is reproduced and resolved (as it appears the issue originating from XMemcached library), I am thinking into creating the patch version, where the default client would be switched back to AWS' spymemcached.
Do let me know if you would find the patch useful for you, until the issue is resolved?

davidkarlsen commented 4 years ago

See https://github.com/killme2008/xmemcached/issues/107 - I could get it working, however - it might be useful for clients to select implementation ("static", "aws" or "appengine") vs ("xmemcached", "spymemcached", "aws", "appengine") - is that what you were thinking about?

davidkarlsen commented 4 years ago

Another option is to let clients provide a bean themselves, and conditionally kick in the autoconfig?

igorbolic commented 4 years ago

See killme2008/xmemcached#107 - I could get it working,

So it does work with new mcrouter configuration. Although I'm not familiar with mcrouter and if the routing actually works without pools definition. Strange thing is that it used to work with SpyMemcached, and not with XMemcached.

however - it might be useful for clients to select implementation ("static", "aws" or "appengine") vs ("xmemcached", "spymemcached", "aws", "appengine") - is that what you were thinking about?

Regarding the patch, you wouldn't really need to change anything with your config (if you are relying on the auto-configuration), updating the library version would opt to using SpyMemcached as the default client. The provider option would remain the same e.g. static where you provide your list of memcache server(s), aws or appengine.

Another option is to let clients provide a bean themselves, and conditionally kick in the autoconfig?

This is how it works from the first version of library. If you configure your own CacheManager bean, auto-configuration will be skipped. So no changes regarding this functionality.

Could you also paste your Spring Boot configuration with the library properties you are using. I will try to find some time to debug it, and find out what might be causing the issue.

davidkarlsen commented 4 years ago

The difference lies in the libs, xmemcached does a touch as well.

igorbolic commented 4 years ago

The difference lies in the libs, xmemcached does a touch as well.

Yes, from the interface perspective XMemcached and SpyMemcached are quite similar, in the implementation they do differ.

If 2.0.0 works for you with mcrouter I would close this issue. I'll create a new issue for the patched version today, which would allow a user of the library to choose SpyMemcached and XMemcached client (if for some reason they prefer one over the other).