You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by vikash32504 <gi...@git.apache.org> on 2016/04/18 23:26:12 UTC

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

GitHub user vikash32504 opened a pull request:

    https://github.com/apache/cxf/pull/132

    3.1.x fixes - CXF-6454

    CXF-6454 - Orphaned JMS connections created in endless loop
    Changing the code to limit the number of retries to a pre-defined value. It can be configured as maxNoOfRetries in JMS Enpoint. 
    Adding another property to make the sleep time configurable. We were seeing high CPU consumption because of low sleep interval time. 

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/vikash32504/cxf 3.1.x-fixes

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cxf/pull/132.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #132
    
----
commit 9b2e894a04fd40ab0178c9f752a760a1dca0fec4
Author: Vikash Kumar <vi...@users.noreply.github.com>
Date:   2016-04-18T21:17:23Z

    Update JMSEndpoint.java

commit c3b98d5602db868e713762279e5c460e18f582c5
Author: Vikash Kumar <vi...@users.noreply.github.com>
Date:   2016-04-18T21:21:06Z

    Update JMSDestination.java

commit 9a2e9e14e758a18b079cca75807e3dd754df6c21
Author: Vikash Kumar <vi...@users.noreply.github.com>
Date:   2016-04-18T21:22:53Z

    Update JMSConfiguration.java

commit 744c4a1d995338fd8703f5baea7002c469035411
Author: Vikash Kumar <vi...@users.noreply.github.com>
Date:   2016-04-18T21:24:28Z

    Update JMSConfigFactory.java

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by dhpatel27 <gi...@git.apache.org>.
Github user dhpatel27 commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-213021417
  
    @cschneider We are also trying to address the 2nd point mentioned in mentioned in CXF Bug CXF-6454, I think we should still try to get rid of infinite loop problem as well. We are trying to give a feature of maxRetries as well so that customers can configure on their own as per their needs. Thus, I would still suggest we put a loop controller with maxRetries as in the original code from this pull request. Further there are 2 problems in addition
    1. If a thread interrupts the loop, you are not cleaning up the resources done in method deactivate(). It simply is coming out without cleaning up the resources.
    2. We are still not giving control to the customer using this feature to control the maxRetries and coming out of the loop without interrupting the thread.
    3. Also, in addition to that, Thread.sleep() should be put in try block before deactivate as done in original code in this pull request since it does not make sense to retry immediately the JMS Connection. Thread.sleep will allow a grace period for Queue Mgr to come up and thus, we should keep Thread.sleep before deactivate and put maxRetries back in loop to come out of the endless loop


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-213348723
  
    I do not yet see a good case for limiting the number of retries as the cxf endpoint or client would be unusable at this point. Can you elaborate how the max retries would help in practice? Especially what would happen after the retries are stopped. Btw. This might be something we could discuss on IRC. I am always in the [channel #apache-cxf](https://cxf.apache.org/mailing-lists.html).
    
    1. Not sure I understand this case. Can you elaborate the steps that would happen?
    2. I think the retry thread should run as long as the CXF endpoint is active. My intention is that you can simply call shutdown on the Destination which ends the retry thread. Interrupting a thread is not such a good idea to achieve this.
    3. I agree to have the sleep there. I wrongly thought that this code would also be run on the first connect.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-212920926
  
    I think the max retries does not really solve the problem with the InvalidClientIdException. 
    I have pushed an alternative solution with just retryInterval and special handling of the exception to
    the CXF branch https://github.com/apache/cxf/tree/CXF-6454. Can you try if this fixes your problem?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request #132: 3.1.x fixes - CXF-6454

Posted by vikash32504 <gi...@git.apache.org>.
Github user vikash32504 closed the pull request at:

    https://github.com/apache/cxf/pull/132


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by tomitribe-dev <gi...@git.apache.org>.
Github user tomitribe-dev commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-218430587
  
    Build triggered. sha1 is merged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by tomitribe-dev <gi...@git.apache.org>.
Github user tomitribe-dev commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-218430942
  
    Build started sha1 is merged.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by tomitribe-dev <gi...@git.apache.org>.
Github user tomitribe-dev commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-218432457
  
    Build finished. 717 tests run, 2 skipped, 0 failed.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by cschneider <gi...@git.apache.org>.
Github user cschneider commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-212747038
  
    Will look into it. The tests look good already. 
    I will need to squash the commits into one commit but that is fine. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by vikash32504 <gi...@git.apache.org>.
Github user vikash32504 commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-212613602
  
    @cschneider - can you please check this. If this fix looks fine to you. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by dhpatel27 <gi...@git.apache.org>.
Github user dhpatel27 commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-217244517
  
    We are hitting critical issues due to this bug in our applications


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by dhpatel27 <gi...@git.apache.org>.
Github user dhpatel27 commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-213663398
  
    @cschneider The use case for point-1 mentioned by me above is when the application wants the flexibility to stop the retries after certain number of attempts if the Connection couldnt be established to JMS queue managers or JMS Destinations itself. The client may not like to continue the retry infinite times due to the CPU Spikes that they can experience unnecessarily after every completion of thread.sleep. Also, this situation becomes worst if a single JVM has >50 JMS Destination Listeners defined and a single physical host has multiple such JVMs. In such scenario, client would still see high CPU when it continues to do infinite retries. Thus it makes sense to give flexibility to client in terms of exposing both retries and retryinterval.
    
    Also for point 2, I was trying to avoid need for client to put custom solution to call explicitly shutdown on JMSDestination as well. ALso, if you note, shutdown variable is proned to race condition since its not defined volatile. If you call shutdown from another Thread, and the retry thread is still looping in another thread, the change in the value of shutdown variable may not be instantaenously visible to other thread.
    Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] cxf pull request: 3.1.x fixes - CXF-6454

Posted by dhpatel27 <gi...@git.apache.org>.
Github user dhpatel27 commented on the pull request:

    https://github.com/apache/cxf/pull/132#issuecomment-217244341
  
    hi @cschneider Can you please comment on my last comment. If you are fine then we would like to go ahead with the original fix provided by @vikash32504 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---