[WIP] Put katello event queue handler back into Dynflow #11363
Conversation
| suspend do | ||
| katello_event = ::Katello::EventQueue.next_event(event.event_type, event.object_id) | ||
| if katello_event | ||
| handler = ::Katello::EventMonitor::PollerThread.new |
There was a problem hiding this comment.
PollerThread is the original name of the class. It does not spawn any threads
| def plan | ||
| plan_self | ||
| end | ||
|
|
There was a problem hiding this comment.
You can drop this, there's a general blanket implementation in dynflow itself that covers this.
| end | ||
|
|
||
| def run(event = nil) | ||
| match(event, |
There was a problem hiding this comment.
A good old case should work here as well and make it look more ruby-like.
| katello_event = ::Katello::EventQueue.oldest_runnable_event | ||
| if katello_event | ||
| # Passing this data to run avoids extra db querying | ||
| plan_event(Handle[katello_event.event_type, katello_event.object_id]) |
There was a problem hiding this comment.
Why not process the event straight away? I mean this works, but it adds an unnecessary round trip
| @@ -77,6 +77,14 @@ class Engine < ::Rails::Engine | |||
| ForemanTasks.dynflow.eager_load_actions! | |||
| end | |||
|
|
|||
| initializer "katello.start_katello_events", before: :finisher_hook do | |||
| unless Rails.env.test? | |||
| ForemanTasks.dynflow.config.post_executor_init do |world| | |||
There was a problem hiding this comment.
if the action stops/pauses/whatever, the event queue won't be processed until the dynflow-sidekiq@orchestrator service is restarted. We should pay extra care to make the action essential immortal
| module Actions | ||
| module Katello | ||
| module EventQueue | ||
| class Monitor < Actions::EntryAction |
There was a problem hiding this comment.
The default rescue_strategy is to pause the action. Iirc paused actions still hold the singleton lock. Using any of the other two strategies which would cause the lock to be unlocked on failure would probably be better idea for more hands-off operation.
| end | ||
|
|
||
| Poll = Algebrick.atom | ||
|
|
There was a problem hiding this comment.
Unless stated otherwise, actions use the default queue. The default sidekiq queue is already used for almost everything, is it ok that event processing might be delayed?
There was a problem hiding this comment.
It should be OK in combination with your prior point: instead of using a Poll -> Handle ping-pong the Poll will fully drain the queue in a loop which can be broken early with if world.terminating? to make sure shutdown isn't blocked.
| module Actions | ||
| module Katello | ||
| module EventQueue | ||
| class Monitor < Actions::EntryAction |
There was a problem hiding this comment.
Iirc foreman-maintain performs checks for long running tasks before upgrade, we'll probably need to whitelist this new action in there.
jturel
force-pushed
the
katello_event_queue_dynflow
branch
from
20e9a1c to
efdf968
Compare
|
Hey @jturel, thanks for the PR. We're conducting a review of the open PRs this afternoon and were just passing through. Please ping us when you'd like a review! |
What are the changes introduced in this pull request?
I decided to leverage AI to answer an age-old question: how can the katello event queue without dynflow? After an intrepid vibe coding session an answer emerged: run the katello event queue with dynflow. The code you are about to read is 100% AI generated.
Jokes aside, using this approach or one like it will stop running the queue in background threads in the web process which is an improvement. Not yet sure if there are any practical downsides to the changes here, but it is functional!
Considerations taken when implementing this change?
Trust AI completely
What are the testing steps for this pull request?
TBD