-
Notifications
You must be signed in to change notification settings - Fork 210
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
rabbitmq queue monitor + queue stability enhancements #570
Conversation
opc_ctl = "/opt/opscode/bin/private-chef-ctl" | ||
opc_username = OmnibusHelper.new(node).ownership['owner'] | ||
rmq_ctl_chpost = "/opt/opscode/embedded/bin/chpst -u #{opc_username} -U #{opc_username} #{rmq_ctl}" | ||
rmq_plugins_chpost = "/opt/opscode/embedded/bin/chpst -u #{opc_username} -U #{opc_username} #{rmq_plugins}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that the _chpost
extension is used above, but part of me feels that this is a typo for _chpst
that has lived for a while.
{ok, "404", _, _} -> | ||
lager:info("RabbitMQ max-length policy not set"), | ||
undefined; | ||
Resp -> lager:error("Unknown response from RabbitMQ management console: ~p", [Resp]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation here doesn't match the rest of the cases.
This looks good, with some relatively minor comments above. My only large area of concern is the volume of requests this will be handling. As you pointed out the timeout option is a bit of a red herring-- realistically if we ever hit that timeout we're already going to be in a really bad state. This is very lightweight as implemented, and I'm probably being overly paranoid - I'd feel better if we had decent integrated load suite that didn't require spinning up a ton of AWS nodes. |
thanks for the review @marcparadise @stevendanna, I'll clean things up on Monday. |
dc45a0a
to
40a72b1
Compare
edfb9d2
to
8701211
Compare
ping @chef/lob |
@@ -116,6 +116,54 @@ | |||
default['private_chef']['rabbitmq']['consumer_id'] = 'hotsauce' | |||
default['private_chef']['rabbitmq']['env_path'] = "/opt/opscode/bin:/opt/opscode/embedded/bin:/usr/bin:/bin" | |||
|
|||
default['private_chef']['rabbitmq']['ssl_versions'] = "'tlsv1.2', 'tlsv1.1'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super minor nitpick, would this be better expressed as an Array
?
👍 |
ChangeLog-Entry: [omnibus] enable RabbitMQ Management Plugin - queue monitor review cleanup - set prevent_erchef_startup_on_full_capacity default to false - change rabbitmq ssl_versions att to an array - queue monitor queue_at_capacity wasn't getting set upon startup POODLE fix for rabbit mgmt console: From https://www.rabbitmq.com/mochiweb.html "For convenience, if you do not specify ssl_opts then rabbitmq-mochiweb will use the same options as the main RabbitMQ server does for AMQP over SSL, but with client certificate verification turned off. If you wish to use client certificate verification, specify ssl_opts explicitly."
d3a0b22
to
4a4b05c
Compare
rabbitmq queue monitor + queue stability enhancements
This PR adds several enhancements for RabbitMQ stability.
max-length
policy is set on the/analytics
exchange15672
. Arabbitmgmt
user is created to access this console.chef_wm_actions_queue_monitoring.erl
). This gen_server periodically connects to the management endpoint to check max-length and current length of the analytics queue. If the queue is at > 80% capacity, a warning is issued via lager. If the queue reaches 100% capacity, toggle a "queue at capacity" flag until messages have been read/purged from the queue. Code that publishes messages to RabbitMQ can check the state of the "queue at capacity" flag before enqueuing._status
endpoint.More details are available in
chef_wm_actions_queue_monitoring.erl
.Notes
This PR adds the PropER testing tool, which is _GPL V3. HOWEVER, this is a _test-only dependency.
SSL for the RabbitMQ Management has been configured to only accept
tlsv1.2
andtlsv1.1
(POODLE, BEAST) via:TODO:
chef_wm_status_tests.erl