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

hystrix javanica aspectj compile time weaving #907

Open
tol opened this Issue Sep 23, 2015 · 35 comments

Comments

Projects
None yet
9 participants
@tol
Copy link

tol commented Sep 23, 2015

Hi,

We try to use javanica with compile time weaving.
Our maven configuration is :

    <aspectj.version>1.6.11</aspectj.version>
        <maven.compiler.source>1.6</maven.compiler.source>
        <maven.compiler.target>1.6</maven.compiler.target>


<plugin>
                    <groupId>org.codehaus.mojo</groupId>
                    <artifactId>aspectj-maven-plugin</artifactId>
                    <version>1.4</version>
                    <executions>
                        <execution>
                            <phase>process-sources</phase>
                            <goals>
                                <goal>compile</goal>
                                <goal>test-compile</goal>
                            </goals>
                        </execution>
                    </executions>
                    <configuration>                     
                        <aspectLibraries>
                            <aspectLibrary>
                                <groupId>com.netflix.hystrix</groupId>
                                <artifactId>hystrix-javanica</artifactId>
                            </aspectLibrary>
                        </aspectLibraries>
                        <!--<outxml>true</outxml>
                        <ajdtBuildDefFile>build.ajproperties</ajdtBuildDefFile>
                        <verbose>true</verbose>
                        <aspectDirectory>src/main/java</aspectDirectory>-->
                        <source>${maven.compiler.source}</source>
                        <target>${maven.compiler.source}</target>
                    </configuration>
                </plugin>```

```java
@HystrixCommand
    public String doWork() throws Exception {

At run time we get the following error:
Exception in thread "main" java.lang.NoSuchMethodError: com.netflix.hystrix.contrib.javanica.aop.aspectj.HystrixCommandAspect.aspectOf()Lcom/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect;

Is there a way to have javanica aspects applied at compile time?

Thank You

@davidkarlsen

This comment has been minimized.

Copy link
Contributor

davidkarlsen commented Sep 23, 2015

Have you tried upgrading to maven-aspectj-plugin 1.8 and aspectj 1.8.7?

@tol

This comment has been minimized.

Copy link
Author

tol commented Sep 23, 2015

yes, we got the same error

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Sep 23, 2015

I'll take a look as soon as can

@tol

This comment has been minimized.

Copy link
Author

tol commented Sep 23, 2015

we tried to extract HystrixCommandAspect and use it at compile time .
we got into a infinite loop in the around advice. It seems that when it reaches com.netflix.hystrix.contrib.javanica.command.MethodExecutionAction mthod execute ( result = m.invoke(o, args); ) the aspect is invoked again until the thread pool is exhausted.

The final error is:
Caused by: java.util.concurrent.RejectedExecutionException: Task java.util.concurrent.FutureTask@5e232be7 rejected from java.util.concurrent.ThreadPoolExecutor@57177af9[Running, pool size = 100, active threads = 100, queued tasks = 0, completed tasks = 0]

I hope this helps.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Sep 23, 2015

Thanks for help. Will see
On 23 Sep 2015 12:05 pm, "tol" notifications@github.com wrote:

we tried to extract HystrixCommandAspect and use it at compile time .
we got into a infinite loop in the around advice. It seems that when it
reaches com.netflix.hystrix.contrib.javanica.command.MethodExecutionAction
mthod execute ( result = m.invoke(o, args); ) the aspect is invoked again
until the thread pool is exhausted.

The final error is:
Caused by: java.util.concurrent.RejectedExecutionException: Task
java.util.concurrent.FutureTask@5e232be
https://github.com/java.util.concurrent.FutureTask/Hystrix/commit/5e232be7
rejected from java.util.concurrent.ThreadPoolExecutor@57177af
https://github.com/java.util.concurrent.ThreadPoolExecutor/Hystrix/commit/57177af9[Running,
pool size = 100, active threads = 100, queued tasks = 0, completed tasks =
0]

I hope this helps.


Reply to this email directly or view it on GitHub
#907 (comment).

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Sep 24, 2015

Could you please provide the client code ? Or it's pretty simple, just one method annotated with the annotation ?

@tol

This comment has been minimized.

Copy link
Author

tol commented Sep 24, 2015

Hi, a simple test method, annotated with @HystrixCommand, which is called repeatedly:

public class TestService {
private static final Logger LOG = LoggerFactory.getLogger(TestService.class);

private final boolean shouldFail;
private final int waitTime;

public TestService(boolean shouldFail, int waitTime) {
    this.shouldFail = shouldFail;
    this.waitTime = waitTime;
}

@HystrixCommand
public String doWork() throws Exception {
   System.out.println("TestService, will wait {} millis..."+waitTime);
    Thread.sleep(waitTime);
    if (this.shouldFail) {
        throw new RuntimeException("Test failure");
    }
    return "done";
}

}

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Sep 28, 2015

I found the root cause of problem and started working on it. The problem is evident, ajc transforms byte code of instrument method, thus we can't get method in original state anymore. But there is one trick in ajc that I've used to fix the problem. There is one disadvantage with it 'cause javanica becomes very dependent on certain aspectj version, it means that in a case of any compatibility issues we can't easily increase aspectj version we need to change javanica code also. But this approach looks simple and doesn't require any changes in client api or adding new features to support it. But you need to run java process with the property: -DweavingMode=compile . @tol I didn't have I chance to add tests for all cases, I just checked simple one you provided above. I'll add more tests and see if the solution really works. But for now you can build from my branch and test your cases: https://github.com/dmgcodevil/Hystrix/tree/javanica-iss-907. Thanks

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Oct 13, 2015

@tol I've finished working on support for compile time weaving, but haven't created PL yet, because suddenly I recognized that you cannot you use CTW if an aspect is located in jar and this aspect wasn't compiled using ajc, otherwise you'll get MethodNotFoundException: aspectOf(). If you want to use CTW then you need to copy HystrixCacheAspect and HystrixCommandAspect from javanica source code into your project and compile javanica aspects+your source code files using ajc. If you are ok with it then I'll create PL. BTW, why you don't use LTW, it has same performance I guess. LTW helps to solve this problem but requires javaagent.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Oct 13, 2015

@tol Or I can add task to release sep jar with aspects compiled using ajc. @mattrjacobs is it possible to upload/release two jars: hystrix-javanica-x.y.z.jar and hystrix-javanica-ctw.x.y.z.jar ?
Then you don't need to copy aspects and you will not see MethodNotFoundException: aspectOf() error

@mattrjacobs

This comment has been minimized.

Copy link
Contributor

mattrjacobs commented Oct 14, 2015

@dmgcodevil Nice work finding the issue.

All of the build/deploy pieces are governed by the Gradle script. If you write a PR for a Gradle script change, then I could test that out. I really don't want to add a manual process if it can be avoided.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Oct 14, 2015

@mattrjacobs thanks, I didn't mean do it manually, of cource I'm going to change gradle script. Let me try and I'll encourage you if have some problems with it

@pagrus7

This comment has been minimized.

Copy link

pagrus7 commented Oct 26, 2015

@dmgcodevil Would it not be possible to resolve this issue without requiring another system property, -DweavingMode=compile ?
As far as I see, javanica inspects this property to recognize the CTW mode and use it in if/else blocks.

Now, CTW mode is the only intended usage mode hystrix-javanica-ctw-X.Y.Z artifact, right? Then, the implementation may safely assume CTW usage in the *-ctw artifact, without requiring extra system properties. So javanica user chooses weaving mode at build time by introducing dependency on CTW or non-CTW artifact.

@pagrus7

This comment has been minimized.

Copy link

pagrus7 commented Oct 26, 2015

... also asked spring-cloud-netflix (the downstream project) if they can provide an input on the issue and solution implications.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Oct 26, 2015

@pagrus7 I hear you man but javanica,jar and javanica-ctw.jar share the same code base and I don't want to support two versions of code. I mean, to make ctw jar works without property we need to redesign project structure: create javanica-core module and etc. Actually issue has been solved and PL merged. We can eliminate this property but I don't have a bandwidth for that right now.

@pagrus7

This comment has been minimized.

Copy link

pagrus7 commented Oct 26, 2015

@dmgcodevil Hey, for the sake of discussion completeness, what do you think is the ideal, elegant way of dealing with CTW? I mean if you had a bandwidth - or someone else would be interested in contributing?

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Oct 26, 2015

@pagrus7 Actually it was not easy to implement that. One inconvenience is extra property but I don't think that is a big problem for now. In the future we can remove but now I'm busy with other hystrix issues. Looks like I'm only one contributor in javanica.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Nov 4, 2015

@mattrjacobs, hi, close it please

@mattrjacobs mattrjacobs closed this Nov 4, 2015

@mattrjacobs

This comment has been minimized.

Copy link
Contributor

mattrjacobs commented Nov 11, 2015

I just released 1.4.20 and the publishing process did not push out the new jar you were working on. I'll investigate.

https://bintray.com/netflixoss/maven/Hystrix/1.4.20/#files/com/netflix/hystrix/hystrix-javanica/1.4.20

@mattrjacobs mattrjacobs reopened this Nov 11, 2015

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Nov 11, 2015

Ok, let me know if you will need any help
On 10 Nov 2015 7:10 p.m., "Matt Jacobs" notifications@github.com wrote:

Reopened #907 #907.


Reply to this email directly or view it on GitHub
#907 (comment).

@mattrjacobs

This comment has been minimized.

Copy link
Contributor

mattrjacobs commented Nov 13, 2015

I'm not a Gradle expert, but I think the best way to solve this would be to give hystrix-javanica-ctw its own subproject.

I can start doing this, but wanted to check with you @dmgcodevil, to make sure it sounds reasonable to you.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Nov 13, 2015

@mattrjacobs The vibe now is that in this case hystrix-javanica-ctw will have exactly the same code, to avoid that I need to create three modules: hystrix-javanica-core, hystrix-javanica and hystrix-javanica-ctw. It will be ideal. Now I'm working in onservable feature, once I finish it I'll jump on that

@mattrjacobs

This comment has been minimized.

Copy link
Contributor

mattrjacobs commented Nov 13, 2015

OK, will defer to you. Thanks!

@raffaelespazzoli

This comment has been minimized.

Copy link

raffaelespazzoli commented Jan 18, 2016

I'm having the same issue with 1.4.22.
For basic usage is there a previous version that would work?
Also any ETA on the correction of this issue?

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Apr 12, 2016

Hi @raffaelespazzoli, as @mattrjacobs mentioned, there is an issue with gradle release plugin or something that blocks us to release javanica-ctw jar without having separate module, it's my undestanding. I didn't investigate the issue from gradle perspective so I can't tell you ETA. It would be great if can fix the issue by just tweaking release procedure, if not then I'd estimate it up to 5d. @mattrjacobs did you have a chance to investigate the issue with artifact deployment ?

@mattrjacobs

This comment has been minimized.

Copy link
Contributor

mattrjacobs commented Apr 18, 2016

@dmgcodevil I haven't had any time to make progress on this, so AFAIK it's still in the same state as before.

@tol

This comment has been minimized.

Copy link
Author

tol commented Mar 15, 2017

Hi,

How can I get the hystrix-javanica-ctw?
I am tring this, but it didn't work...

    <dependency>
        <groupId>com.netflix.hystrix</groupId>
        <artifactId>hystrix-javanica-ctw</artifactId>
        <version>1.5.10</version>
    </dependency>

Can you help me please?

@mattrjacobs
@dmgcodevil

@mattrjacobs

This comment has been minimized.

Copy link
Contributor

mattrjacobs commented Mar 15, 2017

I never figured out a way to get Gradle to generate and publish this artifact. Any help would be appreciated.

@brucehvn

This comment has been minimized.

Copy link

brucehvn commented Oct 31, 2017

So, is there any solution to this? The docs for hystrix-javanica still reference using hystrix-javanica-ctw-x.y.z.jar for compile time weaving. Or is there another way to achieve it using the standard jar?

@sahilchdhary

This comment has been minimized.

Copy link

sahilchdhary commented Mar 8, 2018

I am having the similar issue. I am getting below error:

Task java.util.concurrent.FutureTask@6c15a847 rejected from java.util.concurrent.ThreadPoolExecutor@29172ba1[Running, pool size = 10, active threads = 10, queued tasks = 0, completed tasks = 0]
	at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2047) ~[?:1.8.0_131]

Even though, there should not be any concurrency issue, as i am submitting only one request. So, ideally only one task should be submitted to the threadpool. But somehow, hystrix seems to be spanning out threads for the same task, until the pool gets exhausted, and the last thread just goes to fallback method, due to the above error.

Could someone help here ?
I am using hystrix-javanica:1.5.12 and spring 4.3.3-RELEASE.

@NarendraSinghBhati

This comment has been minimized.

Copy link

NarendraSinghBhati commented Jun 26, 2018

Mine is simple Java application(No Spring) and I am using the configuration as mentioned @https://stackoverflow.com/questions/50994164/hystrixcommandaspect-resulting-into-nosuchmethoderror-in-plain-java-application

I am also facing the same issue, appreciate any help.

@NarendraSinghBhati

This comment has been minimized.

Copy link

NarendraSinghBhati commented Jun 27, 2018

I tried with Load Time Weaving, but end up into the same issue.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Jun 27, 2018

could you please create a github repo with a small project that illustrates the issue?

@NarendraSinghBhati

This comment has been minimized.

Copy link

NarendraSinghBhati commented Jul 3, 2018

Where can I find hystrix-javanica-ctw-x.y.z.jar? I cannot see it in https://mvnrepository.com.

@dmgcodevil

This comment has been minimized.

Copy link
Contributor

dmgcodevil commented Jul 3, 2018

download javanica src and build it, the ctw artifact should be in the target folder, it's generated as a part of regular build process but it's not published

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.