Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark committed_capacity field for removal #12041

Merged
merged 1 commit into from
Apr 18, 2022

Conversation

AlanCoding
Copy link
Member

SUMMARY

Explaining why this field exists:

This app used to use celery and RabbitMQ as the messaging system.

In that old design, the instance groups corresponded to literal queues in RabbitMQ. When the task manager dispatched a task, it submitted it to its instance group queue. This meant that we had no control over which node (inside of the group) received the job, because all nodes in the group would be pulling from that queue.

This was redesigned entirely, we don't use RabbitMQ, replacing it with the postgres message bus. Instead of having queues for instance groups, every instance had a queue. Instead of the task manager selecting an instance group alone, it now selects the instance_group, controller_node, and execution_node, and sends the message to the controller_node queue.

The construct of "committed" capacity is for jobs with "waiting" status which have already been submitted to the instance group queue (the type of queue that no longer exists).

In the modern system, all capacity use is committed to a particular instance. The consumed capacity for an instance group is just the consumed capacity of the instances in that group, added up.

This field is archaic at this point.

OPTIONS with this change

            "committed_capacity": {
                "type": "field",
                "label": "Committed capacity",
                "help_text": "This resource has been deprecated and will be removed in a future release",
                "filterable": false
            },
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • API

Copy link
Member

@kdelee kdelee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@@ -4873,7 +4873,7 @@ class InstanceGroupSerializer(BaseSerializer):

show_capabilities = ['edit', 'delete']

committed_capacity = serializers.SerializerMethodField()
committed_capacity = serializers.SerializerMethodField(help_text=_('This resource has been deprecated and will be removed in a future release'))
consumed_capacity = serializers.SerializerMethodField()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw consumed_capacity also uses capacity_values in the ha.py that we've talked about axing. We should think about if we can deprecate that too (unless we want to do some other re-implementation w/o the capacity_values function)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should want to remove the IG consumed_capacity. Ultimately, if we cache task_impact for unified jobs, then we can do the summation in the database by using an efficient Django annotate.

@AlanCoding AlanCoding merged commit 9059dce into ansible:devel Apr 18, 2022
@AlanCoding AlanCoding deleted the commitment_problems branch April 22, 2022 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants