Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6.1
    • Fix Version/s: 1.7
    • Component/s: Ant Integration
    • Labels:
      None

      Description

      Cactus does not allow to define memory used by application server, then some tests fail and error "OutOfMemoryError" occur.

      1. CACTUS_158.patch
        2 kB
        Matheus Piai Bianconi

        Activity

        Hide
        Matheus Piai Bianconi added a comment -

        PS: We are working on a solution and will send a patch later (even though this solution may not be generic).

        Show
        Matheus Piai Bianconi added a comment - PS: We are working on a solution and will send a patch later (even though this solution may not be generic).
        Hide
        Felipe Leme added a comment -

        Vincent,

        I haven't seen the patch yet, but I have the feeling there are 2 places where we should consider a generic approach:

        1.This issue might happens in any container, not only JBoss, so the fix should go into the super-class of all container tasks (I think it should be defined on AbstractContainerJava or somthing like that). Note also that this issue might be appliable to Cargo as well.

        2.The Java Ant tasks has a method to set the max memory; using that method should fix that specific problem, but we could be generic here too, allowing arbitrary JVM args to be passed to the container.

        Once we define a way to fix it, I could implement the changes. So, any thoughts?

        – Felipe

        Show
        Felipe Leme added a comment - Vincent, I haven't seen the patch yet, but I have the feeling there are 2 places where we should consider a generic approach: 1.This issue might happens in any container, not only JBoss, so the fix should go into the super-class of all container tasks (I think it should be defined on AbstractContainerJava or somthing like that). Note also that this issue might be appliable to Cargo as well. 2.The Java Ant tasks has a method to set the max memory; using that method should fix that specific problem, but we could be generic here too, allowing arbitrary JVM args to be passed to the container. Once we define a way to fix it, I could implement the changes. So, any thoughts? – Felipe
        Hide
        Vincent Massol added a comment -

        Fyi, just done that for Cargo. See http://jira.codehaus.org/browse/CARGO-89

        Show
        Vincent Massol added a comment - Fyi, just done that for Cargo. See http://jira.codehaus.org/browse/CARGO-89
        Hide
        Matheus Piai Bianconi added a comment -

        This is the patch - it hasn't been fully tested yet, but it should work.

        Show
        Matheus Piai Bianconi added a comment - This is the patch - it hasn't been fully tested yet, but it should work.
        Hide
        Vincent Massol added a comment -

        Hi Matheus,

        Just a quick question: why don't you call java.setJvmargs() instead of calling java.createJvmarg().setValue(...)? Does your solution work when you have to pass several JVM arguments?

        I'm curious because I've used setJvmargs() for Cargo and it is working fine.

        Thanks

        Show
        Vincent Massol added a comment - Hi Matheus, Just a quick question: why don't you call java.setJvmargs() instead of calling java.createJvmarg().setValue(...)? Does your solution work when you have to pass several JVM arguments? I'm curious because I've used setJvmargs() for Cargo and it is working fine. Thanks
        Hide
        Felipe Leme added a comment -

        Vincent,

        What about the maven plugin? Should we create a generic property for all containers (like cactus.containers.jvmArgs) or one per container (cactus.jboss3x.jvmArgs, cactus.tomcat5.jvmArgs, etc...)?

        Show
        Felipe Leme added a comment - Vincent, What about the maven plugin? Should we create a generic property for all containers (like cactus.containers.jvmArgs) or one per container (cactus.jboss3x.jvmArgs, cactus.tomcat5.jvmArgs, etc...)?
        Hide
        Felipe Leme added a comment -

        Vincent,

        I tried the setJvmArgs, but got the following message:

        [cactus] The jvmargs attribute is deprecated. Please use nested jvmarg elemets.

        Did you get that with Cargo too?

        I haven't found a @deprecated in the Ant's API, so I think this message is a bug - I think it was meant only to the Ant task, not the java call (i.e., it should be displayed only when jvmargs is passed as an attribute to the java task).

        – Felipe

        Show
        Felipe Leme added a comment - Vincent, I tried the setJvmArgs, but got the following message: [cactus] The jvmargs attribute is deprecated. Please use nested jvmarg elemets. Did you get that with Cargo too? I haven't found a @deprecated in the Ant's API, so I think this message is a bug - I think it was meant only to the Ant task, not the java call (i.e., it should be displayed only when jvmargs is passed as an attribute to the java task). – Felipe
        Hide
        Vincent Massol added a comment -

        Felipe,

        1/ About the deprecation. This should only happen if you use <java jvmarsg="...">. You shouldn't get it at the Java API level (at least I did not notice it when I've done the change on Cargo - Maybe I've just missed it).

        2/ About the maven plugin. I think having one per container is the most flexible and would be useful to run cactus tests on several containers at once. You could always have a generic one and then have each container-specific one default to the generic one.

        thanks

        Show
        Vincent Massol added a comment - Felipe, 1/ About the deprecation. This should only happen if you use <java jvmarsg="...">. You shouldn't get it at the Java API level (at least I did not notice it when I've done the change on Cargo - Maybe I've just missed it). 2/ About the maven plugin. I think having one per container is the most flexible and would be useful to run cactus tests on several containers at once. You could always have a generic one and then have each container-specific one default to the generic one. thanks
        Hide
        Felipe Leme added a comment -

        Nevermind, I figured out the solution: calling createJvmarg().setLines() instead of createJvmarg().setValue().

        In fact, that's exactly what the setJvmargs method does:

        public void setJvmargs(String s)

        { log("The jvmargs attribute is deprecated. " + "Please use nested jvmarg elements.", Project.MSG_WARN); cmdl.createVmArgument().setLine(s); }

        public Commandline.Argument createJvmarg()

        { return cmdl.createVmArgument(); }

        So, I fixed that - the only pending issue now is which properties to use on the maven plugin.

        – Felipe

        Show
        Felipe Leme added a comment - Nevermind, I figured out the solution: calling createJvmarg().setLines() instead of createJvmarg().setValue(). In fact, that's exactly what the setJvmargs method does: public void setJvmargs(String s) { log("The jvmargs attribute is deprecated. " + "Please use nested jvmarg elements.", Project.MSG_WARN); cmdl.createVmArgument().setLine(s); } public Commandline.Argument createJvmarg() { return cmdl.createVmArgument(); } So, I fixed that - the only pending issue now is which properties to use on the maven plugin. – Felipe
        Hide
        Vincent Massol added a comment -

        ok, I get it. I've fixed Cargo accordingly. Thanks.

        Show
        Vincent Massol added a comment - ok, I get it. I've fixed Cargo accordingly. Thanks.
        Hide
        Felipe Leme added a comment -

        I also applied the changes to the maven plugin - please let me know if they're fine...

        Show
        Felipe Leme added a comment - I also applied the changes to the maven plugin - please let me know if they're fine...
        Hide
        Vincent Massol added a comment -

        I haven't tried them but sounds good to me!

        Show
        Vincent Massol added a comment - I haven't tried them but sounds good to me!

          People

          • Assignee:
            Felipe Leme
            Reporter:
            Matheus Piai Bianconi
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development