You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by karuturi <gi...@git.apache.org> on 2015/09/21 07:06:08 UTC
[GitHub] cloudstack pull request: Cloudstack-8885 added blocked connection ...
GitHub user karuturi opened a pull request:
https://github.com/apache/cloudstack/pull/857
Cloudstack-8885 added blocked connection listener for rabbitmqeventbus
When rabbitmq connections are blocked(for example when rabbitmq is
server is out of space), all the cloudstack threads which does any
action and publishes to rabbitmq(for example login, launch vm etc.) are
all blocked.
Added a blocked connection listener to handle this and unblock the
parent thread.
also updated rabbitmq amqp client to 3.5.4 from 3.4.2
Testing:
I didnt find a way to write tests for this change as of now.
manually tested that login and other cloudstack apis work when the configured rabbitmq server goes out of space and publish actions are blocked.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/karuturi/cloudstack CLOUDSTACK-8885
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/857.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 #857
----
commit babc60b93a60a88431fd0f3338210f5efb093877
Author: Rajani Karuturi <ra...@citrix.com>
Date: 2015-09-21T04:45:37Z
updating rabbitmq amqp client to 3.5.4 from 3.4.2
commit 9918f8b121dc3f41d113694e5d373276280e486e
Author: Rajani Karuturi <ra...@citrix.com>
Date: 2015-09-21T04:57:48Z
CLOUDSTACK-8885: added blocked connection listener for rabbitmqeventbus
When rabbitmq connections are blocked(for example when rabbitmq is
server is out of space), all the cloudstack threads which does any
action and publishes to rabbitmq(for example login, launch vm etc.) are
all blocked.
Added a blocked connection listener to handle this and unblock the
parent thread.
----
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-142722939
@remibergsma, my LGTM was based on the code. I was not able to fully test it. I checked the diff and it was very small
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/857#discussion_r49947012
--- Diff: plugins/event-bus/rabbitmq/src/org/apache/cloudstack/mom/rabbitmq/RabbitMQEventBus.java ---
@@ -507,6 +512,21 @@ public synchronized boolean stop() {
return true;
}
+ //logic to deal with blocked connection. connections are blocked for example when the rabbitmq server is out of space. https://www.rabbitmq.com/connection-blocked.html
--- End diff --
The first sentence is super-fluent. I would suggest changing the rest to:
```
/**
* connections are blocked for example when the rabbitmq server is out of space. {@link https://www.rabbitmq.com/connection-blocked.html}
*/
```
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/857
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-172329794
one small remark on the comment but code LGTM
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-173197191
Ping @remibergsma @borisroman
I don;t have a way to test it now, but looked at the code - just like @DaanHoogland @wido and @runseb did. I trust the manual tests @karuturi performed. So, LGTM :+1:
Cheers,
Wilder
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by runseb <gi...@git.apache.org>.
Github user runseb commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-144317611
@remibergsma that one is tough to test as @karuturi mentions. so I am with @wido here +1 on code review. Small enough.
We need to think further on how to handle commits like this.
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-144345855
@runseb In a prefect world, one would still run the tests against it to verify they still all pass. Maybe we can execute the basic VM life cycle test (smoke/test_vm_life_cycle.py) and merge if that passes?
```
nosetests --with-marvin --marvin-config=${marvinCfg} -s -a tags=advanced,required_hardware=false smoke/test_vm_life_cycle.py
```
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-142697372
Hi @wido can you please add what you tested so the person to do the other review can focus on something else? 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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-142581607
LGTM
---
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] cloudstack pull request: Cloudstack-8885 added blocked connection ...
Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:
https://github.com/apache/cloudstack/pull/857#issuecomment-172321551
Ping @borisroman @DaanHoogland @wilderrodrigues Can you guys review this please? Seems like a good fix.
---
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.
---