Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Javanica) HystrixCollapser failed while executing #1354

Closed
antonioborondo opened this issue Sep 20, 2016 · 9 comments
Closed

(Javanica) HystrixCollapser failed while executing #1354

antonioborondo opened this issue Sep 20, 2016 · 9 comments

Comments

@antonioborondo
Copy link

Running the following code:

@HystrixCollapser(batchMethod = "getUsersTest")
public String getUserTest(String id) {
  return null;
}

@HystrixCommand
public List<String> getUsersTest(List<String> ids) {
  List<String> result = new ArrayList<>();
  for(String id : ids) {
      result.add("User " + id);
  }
  return result;
}

getUserTest("1");

HystrixCollapser throws the following error:

java.lang.RuntimeException: CommandCollapser HystrixCollapser failed while executing.

    at com.netflix.hystrix.HystrixCollapser.execute(HystrixCollapser.java:438)
    at com.netflix.hystrix.contrib.javanica.command.CommandExecutor.execute(CommandExecutor.java:52)
    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect.methodsAnnotatedWithHystrixCommand(HystrixCommandAspect.java:90)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:95)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
    at java.lang.reflect.Method.invoke(Method.java:508)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:621)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:610)
    at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:68)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:208)
...

If I change the implementation to return an Observable like in your documentation example:

@HystrixCollapser(batchMethod = "getUsersTest")
public Observable<String> getUserTest(String id) {
  return null;
}

@HystrixCommand
public List<String> getUsersTest(List<String> ids) {
  List<String> result = new ArrayList<>();
  for(String id : ids) {
      result.add("User " + id);
  }
  return result;
}

getUserTest("1");

HystrixCollapser throws the following exception:

java.lang.IllegalStateException: Return type of batch method must be java.util.List parametrized with corresponding type: expected (java.util.List<class rx.Observable>)getUsersTest(java.util.List<class java.lang.String>), but it's class java.lang.String

    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect$CollapserMetaHolderFactory.create(HystrixCommandAspect.java:164)
    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect$MetaHolderFactory.create(HystrixCommandAspect.java:106)
    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect.methodsAnnotatedWithHystrixCommand(HystrixCommandAspect.java:84)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:95)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
    at java.lang.reflect.Method.invoke(Method.java:508)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:621)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:610)
    at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:68)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:208)
...

Could you tell me what is wrong in my implementation? Thanks in advance!

@mattrjacobs
Copy link
Contributor

@dmgcodevil Could you take a look at this one? I'm familiar with debugging vanilla Hystrix collapsing, but not with the Javanica variants

@mattrjacobs mattrjacobs changed the title HystrixCollapser failed while executing (Javanica) HystrixCollapser failed while executing Sep 21, 2016
@dmgcodevil
Copy link
Contributor

@antonioborondo
Copy link
Author

I have already managed to make it works returning String and Future just adding this:

// Initialize Hystrix context
HystrixRequestContext context = HystrixRequestContext.initializeContext();

// HystrixCollapser calls here...

// Shutdown Hystrix context
context.shutdown();

But still it is not working with Observable. I am getting the same error that I showed before:

java.lang.IllegalStateException: Return type of batch method must be java.util.List parametrized with corresponding type: expected (java.util.List<class rx.Observable>)getUsersTest(java.util.List<class java.lang.String>), but it's class java.lang.String

    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect$CollapserMetaHolderFactory.create(HystrixCommandAspect.java:164)
    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect$MetaHolderFactory.create(HystrixCommandAspect.java:106)
    at com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect.methodsAnnotatedWithHystrixCommand(HystrixCommandAspect.java:84)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:95)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:55)
    at java.lang.reflect.Method.invoke(Method.java:508)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:621)
    at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:610)
    at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:68)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
    at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:92)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:208)
...

@dmgcodevil Why do you say that it should return Future if you already have a test working with Observable? https://github.com/Netflix/Hystrix/blob/master/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/collapser/BasicCollapserTest.java#L86

@antonioborondo
Copy link
Author

@dmgcodevil Is the PR #1320 related with my problem returning Observable?

If this is the case, when do you have plans to release a new version including these changes?

@dmgcodevil
Copy link
Contributor

hi, collapser feature for observable was added relatively recently, so it might be not released yet. @mattrjacobs when are you going to make next release with this changes ?

@mattrjacobs
Copy link
Contributor

I've got a couple of other small changes I'd like to get in before the next release. Let me see if I can get to them tomorrow. If not, I'll cut a release with the current tip of master to unblock this.

@mattrjacobs
Copy link
Contributor

Sorry for the delay. I just released v1.5.6 from master, so it should have everything that's been submitted and approved. Please report back if this version fixes your issue

@antonioborondo
Copy link
Author

@mattrjacobs v1.5.6 fix the issue. Thank you very much! 😃

@gaoguofan
Copy link

@HystrixCollapser(batchMethod = "findAll", scope = Scope.GLOBAL, collapserProperties = { @HystrixProperty(name="timerDelayInMilliseconds", value = "1000") }) public BaseDto find(Integer id) { System.out.println(id); return new BaseDto(id, null); }
you can use this 'scope = Scope.GLOBAL',you can try it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants