Only register queue workers using drb for dequeue #19829
Conversation
Now that heartbeating is done through a file uncondidionally this method to add a worker to the drb workers list is only applicable to dequeue
| @drb_dequeue_available ||= | ||
| begin | ||
| server.drb_uri.present? && worker_monitor_drb.respond_to?(:register_worker) | ||
| rescue DRb::DRbError |
There was a problem hiding this comment.
Was there something else in this block before? How would DRb::DRbError be raised? We're not doing anything with drb, right?
There was a problem hiding this comment.
ah, I see, we used to call worker_monitor_drb.register_worker which could raise that exception...
There was a problem hiding this comment.
Right, so in this case I just need some call to the drb object to raise to tell if it's available or not. I used a respond_to? here because that proxies over to the server side without actually doing anything.
There was a problem hiding this comment.
Exactly. I don't think the begin/rescue is needed here.
There was a problem hiding this comment.
(it's needed in the method above that calls register_worker on the drb object)
There was a problem hiding this comment.
This is very much needed I think. respond_to? is proxied to the remote drb object so it will raise rather than return false if the drb server is inaccessible.
I would love a way to tell if the drb server is up that didn't rely on exception handling, do you know of something like that?
There was a problem hiding this comment.
@agrare do you know if there's anything like that?
There was a problem hiding this comment.
My bad. I forgot the drb object was proxied and could raise here. Oh DRb.
There was a problem hiding this comment.
Not that I'm aware of, in miq_fault_tolerant_vim which wraps MiqVim to deal with DRb "issues" we just catch DRb::DRbConnError and retry if the broker is on a new port
| @@ -288,8 +288,6 @@ def heartbeat | |||
| _log.info("#{log_prefix} Synchronizing configuration complete...") | |||
| end | |||
|
|
|||
| register_worker_with_worker_monitor unless MiqEnvironment::Command.is_podified? | |||
There was a problem hiding this comment.
yay, that is_podified? removal must feel good...
| end | ||
| end | ||
| end | ||
|
|
…able The worker monitor only needs drb for queue message prefetch now so only queue workers should be registered. Additionally, make run_single_worker work from the command line (without a server process) by ensuring that we can connect to the DRb server before trying to communicate with it. Fixes ManageIQ#19793
carbonin
force-pushed
the
fix_run_single_worker
branch
from
32bbc51 to
3928c43
Compare
|
Checked commits carbonin/manageiq@ef2d304~...3928c43 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.20.0, and yamllint app/models/miq_server/worker_management/dequeue.rb
|
Workers should only actually attempt to add themselves to the
@workershash if they are queue workers, if they are using drb to fetch queue messages, and the drb server is available. In the past, this process was tied into heartbeating over drb and was thus skipped when using files for heartbeating. After the switch was made to heartbeat to files exclusively, this registration started running even when using therun_single_workerscript from the command line.This lead to issue #19793. This PR fixes the issue by ensuring that workers are only registered when they are going to dequeue messages over drb and only allowing workers to dequeue over drb when the server is actually available.
Additionally because heartbeating is now completely separate from worker registration I moved the
#register_workermethod out of the heartbeat concern and into the dequeue one.