Attempt to fix outbound shutdown error #1572

Merged
merged 2 commits into from Aug 22, 2016

Conversation

Projects
None yet
3 participants
@baudehlo
Collaborator

baudehlo commented Aug 19, 2016

This is just a belt and braces attempt to stop Haraka shutting down from the error @ricardopolo reported here: #1570

If it works: Fixes #1570

@ricardopolo

This comment has been minimized.

Show comment
Hide comment
@ricardopolo

ricardopolo Aug 21, 2016

Contributor

In my testing it works 👍 :shipit:

Contributor

ricardopolo commented Aug 21, 2016

In my testing it works 👍 :shipit:

@ricardopolo

This comment has been minimized.

Show comment
Hide comment
@ricardopolo

ricardopolo Aug 21, 2016

Contributor

I have some bad news... after more intensive testing I am seeing this exception.

image

I think the PR did something, the old error does not appear. But we have this new one.

Contributor

ricardopolo commented Aug 21, 2016

I have some bad news... after more intensive testing I am seeing this exception.

image

I think the PR did something, the old error does not appear. But we have this new one.

@baudehlo

This comment has been minimized.

Show comment
Hide comment
@baudehlo

baudehlo Aug 21, 2016

Collaborator

Right, but does the server shut down after that?

On Aug 21, 2016, at 4:22 PM, Ricardo Polo notifications@github.com wrote:

I have some bad news... after more intensive testing I am seeing this exception.

I think the PR did something, the old error does not appear. But we have this new one


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Collaborator

baudehlo commented Aug 21, 2016

Right, but does the server shut down after that?

On Aug 21, 2016, at 4:22 PM, Ricardo Polo notifications@github.com wrote:

I have some bad news... after more intensive testing I am seeing this exception.

I think the PR did something, the old error does not appear. But we have this new one


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@ricardopolo

This comment has been minimized.

Show comment
Hide comment
@ricardopolo

ricardopolo Aug 21, 2016

Contributor

Not @baudehlo the server did not shutdown, just show the error but continues working

Contributor

ricardopolo commented Aug 21, 2016

Not @baudehlo the server did not shutdown, just show the error but continues working

@baudehlo

This comment has been minimized.

Show comment
Hide comment
@baudehlo

baudehlo Aug 21, 2016

Collaborator

Yes that's expected.

On Sun, Aug 21, 2016 at 4:49 PM, Ricardo Polo notifications@github.com
wrote:

Not @baudehlo https://github.com/baudehlo the server did not shutdown,
just show the error but continues working


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1572 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAobY8xjnA3_v4qkwJgymr-29eDQ8-8eks5qiLnTgaJpZM4Jo7qD
.

Collaborator

baudehlo commented Aug 21, 2016

Yes that's expected.

On Sun, Aug 21, 2016 at 4:49 PM, Ricardo Polo notifications@github.com
wrote:

Not @baudehlo https://github.com/baudehlo the server did not shutdown,
just show the error but continues working


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1572 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAobY8xjnA3_v4qkwJgymr-29eDQ8-8eks5qiLnTgaJpZM4Jo7qD
.

@ricardopolo

This comment has been minimized.

Show comment
Hide comment
@ricardopolo

ricardopolo Aug 21, 2016

Contributor

Nice. But if this is a expected behavior I would change the log from Warn to Info.

I dont think is nice to have lots of warns in the logs when are normal operations behaviors.

Contributor

ricardopolo commented Aug 21, 2016

Nice. But if this is a expected behavior I would change the log from Warn to Info.

I dont think is nice to have lots of warns in the logs when are normal operations behaviors.

@baudehlo

This comment has been minimized.

Show comment
Hide comment
@baudehlo

baudehlo Aug 22, 2016

Collaborator

Well it's a warning because the remote end really isn't supposed to do that. It's not very nice of them. But I can lower it to NOTICE perhaps.

Collaborator

baudehlo commented Aug 22, 2016

Well it's a warning because the remote end really isn't supposed to do that. It's not very nice of them. But I can lower it to NOTICE perhaps.

Matt Sergeant
@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 22, 2016

Current coverage is 35.99% (diff: 12.50%)

Merging #1572 into master will decrease coverage by 0.02%

@@             master      #1572   diff @@
==========================================
  Files            26         26          
  Lines          6491       6495     +4   
  Methods         842        843     +1   
  Messages          0          0          
  Branches       1694       1694          
==========================================
  Hits           2338       2338          
- Misses         4153       4157     +4   
  Partials          0          0          

Powered by Codecov. Last update 0b1762f...33198bf

codecov-io commented Aug 22, 2016

Current coverage is 35.99% (diff: 12.50%)

Merging #1572 into master will decrease coverage by 0.02%

@@             master      #1572   diff @@
==========================================
  Files            26         26          
  Lines          6491       6495     +4   
  Methods         842        843     +1   
  Messages          0          0          
  Branches       1694       1694          
==========================================
  Hits           2338       2338          
- Misses         4153       4157     +4   
  Partials          0          0          

Powered by Codecov. Last update 0b1762f...33198bf

@baudehlo

This comment has been minimized.

Show comment
Hide comment
@baudehlo

baudehlo Aug 22, 2016

Collaborator

Following discussion on IRC, I've lowered the default timeout to 50s. It seems most servers have a longer timeout than that, so this should eliminate this warning.

Collaborator

baudehlo commented Aug 22, 2016

Following discussion on IRC, I've lowered the default timeout to 50s. It seems most servers have a longer timeout than that, so this should eliminate this warning.

@baudehlo baudehlo merged commit 7ede33a into master Aug 22, 2016

2 of 3 checks passed

codecov/project 35.99% (-0.03%) compared to 0b1762f
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@msimerson msimerson deleted the outbound_shutdown_error branch Aug 23, 2016

@ricardopolo

This comment has been minimized.

Show comment
Hide comment
@ricardopolo

ricardopolo Sep 22, 2016

Contributor

Sadly the issue continues, I configure pool_timeout to 30 and we sill have this error.

screen shot 2016-09-22 at 2 56 02 pm

So would you change the log level please?
Thanks!

Contributor

ricardopolo commented Sep 22, 2016

Sadly the issue continues, I configure pool_timeout to 30 and we sill have this error.

screen shot 2016-09-22 at 2 56 02 pm

So would you change the log level please?
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment