You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Anshul Gangwar <an...@citrix.com> on 2013/10/03 13:31:50 UTC

Review Request 14467: fix for Invalid SMTP breaks HA

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14467/
-----------------------------------------------------------

Review request for cloudstack and Devdeep Singh.


Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-4792


Repository: cloudstack-git


Description
-------

added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert


Diffs
-----

  server/src/com/cloud/alert/AlertManagerImpl.java c437a8c 

Diff: https://reviews.apache.org/r/14467/diff/


Testing
-------

tested manually on my local setup


Thanks,

Anshul Gangwar


Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Likitha Shetty <li...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14467/#review27181
-----------------------------------------------------------


Changes have been committed to 4.2.
But the patch doesn't clean apply on master, can you please update it accordingly.

- Likitha Shetty


On Oct. 3, 2013, 11:31 a.m., Anshul Gangwar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14467/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2013, 11:31 a.m.)
> 
> 
> Review request for cloudstack and Devdeep Singh.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-4792
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-4792
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java c437a8c 
> 
> Diff: https://reviews.apache.org/r/14467/diff/
> 
> 
> Testing
> -------
> 
> tested manually on my local setup
> 
> 
> Thanks,
> 
> Anshul Gangwar
> 
>


Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Likitha Shetty <li...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14467/#review27180
-----------------------------------------------------------

Ship it!


Ship It!

- Likitha Shetty


On Oct. 3, 2013, 11:31 a.m., Anshul Gangwar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14467/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2013, 11:31 a.m.)
> 
> 
> Review request for cloudstack and Devdeep Singh.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-4792
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-4792
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/alert/AlertManagerImpl.java c437a8c 
> 
> Diff: https://reviews.apache.org/r/14467/diff/
> 
> 
> Testing
> -------
> 
> tested manually on my local setup
> 
> 
> Thanks,
> 
> Anshul Gangwar
> 
>


Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Chip Childers <ch...@sungard.com>.
On Fri, Oct 4, 2013 at 2:17 AM, Anshul Gangwar <an...@citrix.com>wrote:

> On Thursday 03 October 2013 08:44 PM, Chip Childers wrote:
> > On Thu, Oct 03, 2013 at 11:31:50AM +0000, Anshul Gangwar wrote:
> >> Description
> >> -------
> >>
> >> added the connection and socket timeout parameters for SMTP and sending
> message in new thread so that HA doesn't get blocked beacause of hang in
> sending email alert
> > Wouldn't a better approach be to ensure that email sending runs async to
> > critical operations like HA?
>
> This patch is making email sending  async. Currently agent thread first
> send the alert( where emails are sent) and then calls the HA. Can you
> please elaborate more? Is it logic change or something else?
>
> Thanks,
> Anshul
>

You're right.  I was commenting on the description of the review, but
obviously should have looked at the code.

LGTM!

-chip

Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Anshul Gangwar <an...@citrix.com>.
On Thursday 03 October 2013 08:44 PM, Chip Childers wrote:
> On Thu, Oct 03, 2013 at 11:31:50AM +0000, Anshul Gangwar wrote:
>> Description
>> -------
>>
>> added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert
> Wouldn't a better approach be to ensure that email sending runs async to
> critical operations like HA?

This patch is making email sending  async. Currently agent thread first 
send the alert( where emails are sent) and then calls the HA. Can you 
please elaborate more? Is it logic change or something else?

Thanks,
Anshul

Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Chip Childers <ch...@sungard.com>.
On Thu, Oct 03, 2013 at 11:31:50AM +0000, Anshul Gangwar wrote:
> Description
> -------
> 
> added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert

Wouldn't a better approach be to ensure that email sending runs async to
critical operations like HA?

Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Anshul Gangwar <an...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14467/
-----------------------------------------------------------

(Updated Oct. 21, 2013, 8:31 a.m.)


Review request for cloudstack and Devdeep Singh.


Changes
-------

added branch master


Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-4792
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-4792


Repository: cloudstack-git


Description
-------

added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert


Diffs
-----

  server/src/com/cloud/alert/AlertManagerImpl.java ed0c9fd 

Diff: https://reviews.apache.org/r/14467/diff/


Testing
-------

tested manually on my local setup


Thanks,

Anshul Gangwar


Re: Review Request 14467: fix for Invalid SMTP breaks HA

Posted by Anshul Gangwar <an...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14467/
-----------------------------------------------------------

(Updated Oct. 21, 2013, 8:29 a.m.)


Review request for cloudstack and Devdeep Singh.


Changes
-------

updated the patch according to current master


Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-4792
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/CLOUDSTACK-4792


Repository: cloudstack-git


Description
-------

added the connection and socket timeout parameters for SMTP and sending message in new thread so that HA doesn't get blocked beacause of hang in sending email alert


Diffs (updated)
-----

  server/src/com/cloud/alert/AlertManagerImpl.java ed0c9fd 

Diff: https://reviews.apache.org/r/14467/diff/


Testing
-------

tested manually on my local setup


Thanks,

Anshul Gangwar