Problem/Motivation
- Currently, there is a limit to the number of items a queue can have. However, if there are lots of cloud service providers, this number is easily hit. Subsequently, cloud service providers won't be updated.
Issue fork cloud-3267036
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
baldwinlouie commented@yas, I've added support for creating multiple queues using a deriver. I've only added the deriver to the AWS, K8s, Openstack, Terraform, and VMware. I don't think this is needed for cloud_cluster or cloud_cluster_worker.
Comment #4
baldwinlouie commented@yas, setting back to needs work. I need to make queue changes to
K8sCloudStorePlugin.phpas well. Those were caught during Unit Testing.Comment #5
baldwinlouie commentedComment #6
yas@baldwinlouie
Thank you for the update and refactoring. I posted my comment above. Thanks!
Comment #7
baldwinlouie commentedComment #8
yas@baldwinlouie
Thank you for the update. I posted my comment. All-in-all, it looks good to me.
@xiaohua-guan
What do you think?
Comment #9
xiaohua guan commented@yas
It looks good to me. It's a great commit!
Comment #10
yas@xiaohua-guan
Thank you for your review.
@baldwinlouie
I found that we need to refactor the following files for queue ID change:
.tugboat/config.yml</code>cfn/nested/ec2/ami/template.yamlCan you include the refactor for cron job for those files? Thanks!
Comment #11
yas@baldwinlouie @xiaohua-guan
I posted my suggested refactoring. Could you please review the logic?
Also, the default value of e.g. aws_cloud_queue_limit can be
1based on my local testing result.Thanks!
Comment #12
baldwinlouie commented@yas, please review the refactoring. I've extracted the logic that adds items to the queue into two helper methods.
getResourceQueueNameandupdateResourceQueue.I've also updated the following
.tugboat/config.ymlcfn/nested/ec2/ami/template.yamlComment #13
yas@baldwinlouie
Thank you for the update. I reviewed the patch and posted my comments above. Also, could you please change the default value
50to1for<module_name>_queue_limit? Please check: https://git.drupalcode.org/search?search=_queue_limit%3A+50&group_id=2&p... ?Thanks!
Comment #14
baldwinlouie commented@yas, all the requested changes have been made.
Comment #15
yas@baldwinlouie
Can you refactor the rest of code at
AwsCloudUpdateResourcesQueueWorker? Thanks!Comment #16
baldwinlouie commented@yas, please review the updated.
Comment #17
yas@baldwinlouie
Thank you for the update. It looks all good to me.
@xiaohua-guan
Could you please review the patch?
NOTE:
I tested the queue limit by running
watch drush queue-listand found it guarantees that each queue item will be surely processed when we specify queue limit1. It will realize one channel to process all queue item since the queue items will increase only when it becomes zero with our new logic, which means all entities are surely updated. It will create something like a single channel literally like FIFO, that we desire. I thought we didn't have to put multiple queue items for each resource at the same time. Each resource needs to be updated once enough during the update cycle.Comment #18
baldwinlouie commented@yas I've added additional functionality to this patch for queue refactoring.
1. Added a new drush command
drush cloud:queue:cleanup. Example usage of the commanddrush cloud:queue:cleanup --hours=5 --queue_names=queue1,queue2. Delete items from a comma delimited list of queues that are older than 5 hours. This lets you delete from any queue of your choosing, even queues created by other modules.drush cloud:queue:cleanup. This is the default usage and will delete all queue items older than 1 hour. It only deletes from queues created by cloud service providers. Queues created by other modules are ignored.2. Added
try/catcharound all queue processing. This way, if there is an exception, that particular queue item will be booted out of the queue. By doing so, we do not have lingering queue items that can potentially error out.3. When deleting a cloud service provider, delete all their queue items. This will help clean up the table a bit. I've done this in
hook_cloud_config_predelete(). Doing so won't cause any CloudConfigPluginException errors to be thrown mistaken.4. In the
processItemof the Queue Worker plugins, I've abstracted the common function that checks if a service method exists. I'm also checking to see if setting the CloudContext will cause any CloudConfigPluginException errors.Finally, I did not hide the configuration for
*_queue_limitvariable yet. We haven't really testing if count=1 is truly desired yet. If we want to do this, we can create another issue.Comment #19
xiaohua guan commented@yas
It looks good to me. Thanks.
Comment #20
yas@xiaohaua-guan
Thank you for your review.
@baldwinlouie
Thank you for the update. I tested this great patch and looks working well. I'll merge the patch to
4.xand5.x, and close this issue as Fixed.Comment #27
yas