fix: avoid race conditions when removing multiple instance

[pb82]

SUMMARY

Removing multiple instance groups at the same time currently fails. The failure seems to be caused by a race condition, when the UI sends multiple disassociate requests.

Locking the m2m relationship table during each disassociation resolves the problem (likely because this forces all operations to be sequential in the database).

ISSUE TYPE

• Bug, Docs Fix or other nominal change

COMPONENT NAME

• API

AWX VERSION

devel

ADDITIONAL INFORMATION

[AlanCoding]

I would like to avoid cursor.execute calls at all possible costs. There's another problem here, that another solution mechanism already exists. You can see that there's a very different code pattern to this, but it does the same thing of obtaining a row-level lock.

awx/awx/api/views/mixin.py

Lines 78 to 88 in c4d8fdb

	with transaction.atomic():
	instance_groups_queryset = InstanceGroup.objects.select_for_update()
	if self.parent_model is Instance:
	ig_obj = get_object_or_400(instance_groups_queryset, pk=sub_id)
	else:
	# similar to get_parent_object, but selected for update
	parent_filter = {self.lookup_field: self.kwargs.get(self.lookup_field, None)}
	ig_obj = get_object_or_404(instance_groups_queryset, **parent_filter)
	if inst_name not in ig_obj.policy_instance_list:
	ig_obj.policy_instance_list.append(inst_name)
	ig_obj.save(update_fields=['policy_instance_list'])

That implementation lives in InstanceGroupMembershipMixin and is used in:

• InstanceInstanceGroupsList
• InstanceGroupInstanceList

Those 2 views do appear very similar on a surface-level to the view your patch is concerned with, which I believe is the organization instance group list. So I'd like to see more research on whether that mixin can or can't be reused here, and if not, if the ORM code pattern can be used, and whether a fully general implementation is needed for all attach-detach views as you have, or if a more targeted solution can work.

[fosterseth]

had some CI breakage yesterday, rebasing should help

[pb82]

@AlanCoding @chrismeyersfsu I've changed it to a Mixin & select_for_update now, no more full table lock. Could you have another look please?

[AlanCoding]

maybe this needs to be applied to attach as well

[pb82]

I tried it, but it doesn't solve the problem we have with attaching (which is that the order ends up random)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud