Refactor #3675
closed
Using special name to determine RQ worker role can be problematic.
0%
Description
Parsing a special name to determine the role of the PulpWorker can be error prone and lead to failure scenarios that can be difficult to troubleshoot. I ran into this nearly silent failure with the celery code as well and burned a bit of time figuring it out.
rq worker -n 'reserved_resource_worker_1@%h' -w 'pulpcore.tasking.worker.PulpWorker'
Suggest a cleaner, more explicit approach is to create have 2 concrete classes of workers that can determine the queues which are passed to super(). And, perhaps determine its name?
Eg:
rq worker -w 'pulpcore.tasking.worker.ReservedWorker'
rq worker -w 'pulpcore.tasking.worker.ResourceManager'
Would -n <name> still be needed?
Simpler and if the user get's the classpath to the worker wrong, the logged exception will be self explanatory.
[1] https://github.com/pulp/pulp/blob/3.0-dev/pulpcore/pulpcore/tasking/worker.py#L56-L57
Updated by bmbouter almost 6 years ago
How will the class know to name the worker with a _1 versus a _2 when you start multiple worker instances? It's not required that they have numbers in them necessarily, but they do need to be unique.
One option to adopt this change would be for us to start randomizing the names something like worker_fsd768fsd@example.com or similar. That would make running on things like kubernetes probably easier since you can use the same command to start a worker and it will receive a unique name.
Updated by jortel@redhat.com almost 6 years ago
How about using the pid + fqdn for uniqueness in the name?
Updated by bmbouter almost 6 years ago
+1 to using worker_{pid}@{hostname} as the name and have the classes auto-set this.
Updated by dalley almost 6 years ago
- Tracker changed from Issue to Refactor
- % Done set to 0
- Tags Pulp 3 added
Updated by daviddavis over 3 years ago
- Status changed from NEW to CLOSED - CURRENTRELEASE
.