-
Notifications
You must be signed in to change notification settings - Fork 58
Fixes #16863 - Remove qpid event queue #427
Conversation
@@ -66,6 +66,15 @@ def remove_gutterball | |||
end | |||
end | |||
|
|||
def remove_event_queue | |||
queue_present = `qpid-stat -q --ssl-certificate=/etc/pki/katello/qpid_client_striped.crt -b amqps://localhost:5671 | grep :event | wc -l`.chomp.to_i |
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.
How sure are we these paths, ports etc are correct? Would it make sense to use values from the answers file? Also wondering if this should also use the Kafo execute helper.
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.
@chris1984 I believe @ekohl means that these paths are not change proof since the canonical source for the paths is contained within the puppet. If they change, this will break.
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.
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.
Doesn't Kafo migrations have the answer files? While currently I don't think these paths are exposed as parameters now, I wonder if qpid will move to a top level module.
I don't think this is very important right now, but I wanted to raise the question.
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.
@ehelms @ekohl I do know @jlsherrill is working on a PR that will not run already completed steps when the installer runs with --upgrade so this would only get hit once and really by downstream 6.3 most customers should not have this issue/queue anymore. I can always revisit this if needed if the path does change. Let me know how to proceed?
@ekohl This works correctly, since its not removing them all the time it just needs to be moved to post from pre. I have done a few tests and each time with it in post the queue is removed. I can switch to kafo helpers if needed, as far as the paths/ports this is correct. |
I know this was already put in, but I am curious what happens to events still in the queue? |
@ehelms Pavel explains it better than I can so here is a copy/paste from the BZ: Since candlepin sends events to that queue even in 6.2, the queue gets full after some time, causing candlepin either slowed down or raising errors (I can explain why if interested). That has severe impact to whole Satellite, like performance down, or even WebUI returning 503 (passenger serves only rhsm/candlepin requests until they timeout). The upgrade has to delete the queue in either way. It is safe for candlepin, since candlepin sends the events to a qpid exchange that routes the messages to the queue - candlepin itself does not know about presence of the queue at all. |
I'm fine with merging it with hardcoded locations since it's no worse than the current hardcoded locations in pre. |
Same. I just didn't understand the removal fully. So does this remove it for good, as in, the queue is gone for good and is never recreated? Or is this just a one time clear out of the queue? |
@ehelms yes it removes it for good, it is no longer needed since gutterball is removed but the event queue remains and causes performance issues. |
Thanks @chris1984 ! |
Finding on some upgrades the event queue is still present, removing from pre and moving to post.