New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable chef setting for tcp_listen linger option #321

Merged
merged 1 commit into from Nov 24, 2015

Conversation

Projects
None yet
3 participants
@mgosalia

mgosalia commented Nov 19, 2015

No description provided.

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia Nov 19, 2015

@jjasghar could you please review this ?
Reference - https://www.rabbitmq.com/networking.html
grep for "linger".

mgosalia commented Nov 19, 2015

@jjasghar could you please review this ?
Reference - https://www.rabbitmq.com/networking.html
grep for "linger".

@jjasghar

This comment has been minimized.

Show comment
Hide comment
@jjasghar

jjasghar Nov 19, 2015

Collaborator

I'll take a look at this, and do some research on this.

@michaelklishin any thoughts on this option? This is the first i've heard of it.

Collaborator

jjasghar commented Nov 19, 2015

I'll take a look at this, and do some research on this.

@michaelklishin any thoughts on this option? This is the first i've heard of it.

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia Nov 19, 2015

@michaelklishin fyi...I had sent out an email to openstack-operators mailing list as well.
Here is the email - http://lists.openstack.org/pipermail/openstack-operators/2015-November/008878.html

mgosalia commented Nov 19, 2015

@michaelklishin fyi...I had sent out an email to openstack-operators mailing list as well.
Here is the email - http://lists.openstack.org/pipermail/openstack-operators/2015-November/008878.html

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia commented Nov 19, 2015

Thanks @jjasghar

@michaelklishin

This comment has been minimized.

Show comment
Hide comment
@michaelklishin

michaelklishin Nov 19, 2015

Member

Socket lingering is primarily useful to clients but I have no objections to this. Keep in mind that as of 3.6.0 we will be using a TCP acceptor library called Ranch, which has sensible defaults for most, and is fairly strict about rejecting options it doesn't support (in general, the options are nearly identical to gen_tcp, though).

Member

michaelklishin commented Nov 19, 2015

Socket lingering is primarily useful to clients but I have no objections to this. Keep in mind that as of 3.6.0 we will be using a TCP acceptor library called Ranch, which has sensible defaults for most, and is fairly strict about rejecting options it doesn't support (in general, the options are nearly identical to gen_tcp, though).

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia Nov 19, 2015

@michaelklishin do we know for sure that linger {true, 0} is always the default setting in rabbitmq config ?
This documentation [1] does say so.

[1] - https://www.rabbitmq.com/networking.html

mgosalia commented Nov 19, 2015

@michaelklishin do we know for sure that linger {true, 0} is always the default setting in rabbitmq config ?
This documentation [1] does say so.

[1] - https://www.rabbitmq.com/networking.html

@michaelklishin

This comment has been minimized.

Show comment
Hide comment
@michaelklishin
Member

michaelklishin commented Nov 20, 2015

@mgosalia see for yourself:

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia Nov 20, 2015

Thanks @michaelklishin.
@jjasghar is it good to go ? This change will help us at Yahoo to configure the value.

mgosalia commented Nov 20, 2015

Thanks @michaelklishin.
@jjasghar is it good to go ? This change will help us at Yahoo to configure the value.

@jjasghar

This comment has been minimized.

Show comment
Hide comment
@jjasghar

jjasghar Nov 20, 2015

Collaborator

I'm going to run this through our testing today. Any chance you can create an integration/unit test for this? I really don't like adding things to this cookbook without test coverage.

Collaborator

jjasghar commented Nov 20, 2015

I'm going to run this through our testing today. Any chance you can create an integration/unit test for this? I really don't like adding things to this cookbook without test coverage.

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia Nov 20, 2015

@jjasghar sure...will add unit tests. I tried to look for existing tests related to rabbitmq config and did not find any. So, had skipped it. Let me dig more.

mgosalia commented Nov 20, 2015

@jjasghar sure...will add unit tests. I tried to look for existing tests related to rabbitmq config and did not find any. So, had skipped it. Let me dig more.

@jjasghar

This comment has been minimized.

Show comment
Hide comment
@jjasghar

jjasghar Nov 20, 2015

Collaborator

Sounds good, just Unit tests are probably fine. When you get them added I'll go through my release process and get this pushed out.

Collaborator

jjasghar commented Nov 20, 2015

Sounds good, just Unit tests are probably fine. When you get them added I'll go through my release process and get this pushed out.

@mgosalia

This comment has been minimized.

Show comment
Hide comment
@mgosalia

mgosalia Nov 23, 2015

@jjasghar have updated the change with unit tests. Could you please review it again ? Thanks.

mgosalia commented Nov 23, 2015

@jjasghar have updated the change with unit tests. Could you please review it again ? Thanks.

@jjasghar

This comment has been minimized.

Show comment
Hide comment
@jjasghar

jjasghar Nov 23, 2015

Collaborator

Awesome, thanks. I'll run some final tests and push out a new version ASAP.

Collaborator

jjasghar commented Nov 23, 2015

Awesome, thanks. I'll run some final tests and push out a new version ASAP.

jjasghar added a commit that referenced this pull request Nov 24, 2015

Merge pull request #321 from mgosalia/master
Enable chef setting for tcp_listen linger option

@jjasghar jjasghar merged commit f573062 into rabbitmq:master Nov 24, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment