Deadlock when deleting regions

[timobrembeck]

Describe the Bug

Deleting regions via the region form is currently not possble

Steps to Reproduce

1. Go to region form
2. Click on 'Delete region'
3. Wait

Expected Behavior

The region should be deleted

Actual Behavior

Nothing happens, the page is loading forever

Additional Information

Seems to be related to this traceback:

Traceback

In [3]: Region.objects.get(slug="kreis-re").delete()
^C---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/fields/related_descriptors.py:173, in ForwardManyToOneDescriptor.__get__(self, instance, cls)
  172 try:
--> 173     rel_obj = self.field.get_cached_value(instance)
  174 except KeyError:

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/fields/mixins.py:15, in FieldCacheMixin.get_cached_value(self, instance, default)
   14 try:
---> 15     return instance._state.fields_cache[cache_name]
   16 except KeyError:

KeyError: 'language'

During handling of the above exception, another exception occurred:

KeyboardInterrupt                         Traceback (most recent call last)
Cell In[3], line 1
----> 1 Region.objects.get(slug="kreis-re").delete()

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/base.py:967, in Model.delete(self, using, keep_parents)
  965 collector = Collector(using=using)
  966 collector.collect([self], keep_parents=keep_parents)
--> 967 return collector.delete()

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/deletion.py:404, in Collector.delete(self)
  402 for model, obj in self.instances_with_model():
  403     if not model._meta.auto_created:
--> 404         signals.pre_delete.send(
  405             sender=model, instance=obj, using=self.using
  406         )
  408 # fast deletes
  409 for qs in self.fast_deletes:

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/dispatch/dispatcher.py:180, in Signal.send(self, sender, **named)
  177 if not self.receivers or self.sender_receivers_cache.get(sender) is NO_RECEIVERS:
  178     return []
--> 180 return [
  181     (receiver, receiver(signal=self, sender=sender, **named))
  182     for receiver in self._live_receivers(sender)
  183 ]

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/dispatch/dispatcher.py:181, in <listcomp>(.0)
  177 if not self.receivers or self.sender_receivers_cache.get(sender) is NO_RECEIVERS:
  178     return []
  180 return [
--> 181     (receiver, receiver(signal=self, sender=sender, **named))
  182     for receiver in self._live_receivers(sender)
  183 ]

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/linkcheck/listeners.py:189, in instance_pre_delete(sender, instance, **kwargs)
  187 def instance_pre_delete(sender, instance, **kwargs):
  188     instance.linkcheck_deleting = True
--> 189     deleted_url = instance.get_absolute_url()
  190     if deleted_url:
  191         old_urls = Url.objects.filter(url__startswith=deleted_url).exclude(status=False)

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/integreat_cms/cms/models/abstract_content_translation.py:186, in AbstractContentTranslation.get_absolute_url(self)
  159 def get_absolute_url(self):
  160     """
  161     Generates the absolute url of the content translation object
  162
 (...)
  184     :rtype: str
  185     """
--> 186     return self.url_prefix + self.slug + "/"

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/utils/functional.py:48, in cached_property.__get__(self, instance, cls)
   46 if instance is None:
   47     return self
---> 48 res = instance.__dict__[self.name] = self.func(instance)
   49 return res

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/integreat_cms/cms/models/abstract_content_translation.py:124, in AbstractContentTranslation.url_prefix(self)
  106 @cached_property
  107 def url_prefix(self):
  108     """
  109     Generates the prefix of the url of the content translation object
  110
 (...)
  115     :rtype: str
  116     """
  117     return (
  118         "/"
  119         + "/".join(
  120             filter(
  121                 None,
  122                 [
  123                     self.foreign_object.region.slug,
--> 124                     self.language.slug,
  125                     self.url_infix,
  126                 ],
  127             )
  128         )
  129         + "/"
  130     )

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/fields/related_descriptors.py:187, in ForwardManyToOneDescriptor.__get__(self, instance, cls)
  185     rel_obj = None
  186 if rel_obj is None and has_value:
--> 187     rel_obj = self.get_object(instance)
  188     remote_field = self.field.remote_field
  189     # If this is a one-to-one relation, set the reverse accessor
  190     # cache on the related object to the current instance to avoid
  191     # an extra SQL query if it's accessed later on.

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/fields/related_descriptors.py:154, in ForwardManyToOneDescriptor.get_object(self, instance)
  152 qs = self.get_queryset(instance=instance)
  153 # Assuming the database enforces foreign keys, this won't fail.
--> 154 return qs.get(self.field.get_reverse_related_filter(instance))

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/cacheops/query.py:327, in QuerySetMixin.get(self, *args, **kwargs)
  324 else:
  325     qs = self
--> 327 return qs._no_monkey.get(qs, *args, **kwargs)

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/query.py:431, in QuerySet.get(self, *args, **kwargs)
  429     limit = MAX_GET_RESULTS
  430     clone.query.set_limits(high=limit)
--> 431 num = len(clone)
  432 if num == 1:
  433     return clone._result_cache[0]

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/django/db/models/query.py:262, in QuerySet.__len__(self)
  261 def __len__(self):
--> 262     self._fetch_all()
  263     return len(self._result_cache)

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/cacheops/query.py:261, in QuerySetMixin._fetch_all(self)
  259     else:
  260         self._result_cache = list(self._iterable_class(self))
--> 261         self._cache_results(cache_key, self._result_cache)
  263 return self._no_monkey._fetch_all(self)

File /usr/lib/python3.9/contextlib.py:124, in _GeneratorContextManager.__exit__(self, type, value, traceback)
  122 if type is None:
  123     try:
--> 124         next(self.gen)
  125     except StopIteration:
  126         return False

File /opt/integreat-cms/.venv/lib/python3.9/site-packages/cacheops/getset.py:57, in getting(key, cond_dnfs, prefix, lock)
   54 @contextmanager
   55 def getting(key, cond_dnfs, prefix, lock=False):
   56     if not lock:
---> 57         yield _read(key, cond_dnfs, prefix)
   58     else:
   59         locked = False

[david-venhoff]

I cannot reproduce this issue. Did it happen for you only once or every time you tried it?

[david-venhoff]

I tried to reproduce it again and I think that this might not be a deadlock, but just really slow.
For example, this happened when I deleted Augsburg on my local installation:

It seems like the linkcheck library registers a pre-delete handler (And also a post-delete handler) for every page translation (including old versions) and queries its url. That method is quite expensive to call in our case.
https://github.com/DjangoAdminHackers/django-linkcheck/blob/c372147e53f4f65a7c53df7ba2e0f260fc0700d9/linkcheck/listeners.py#L187-L193

However I am not really sure how actionable those information are. I guess, in the worst case, we could also perform deletion of regions in the background? Or we could disable these linkcheck features and implement those manually in a more efficient way (By skipping queries for old translations and maybe batch everything into one db query?)

[timobrembeck]

@david-venhoff good catch!
I think we can safely disable the linkcheck listeners here via

from linkcheck.listeners import disable_listeners
with disable_listeners():
    region.delete()

And just mark internal links to this region as broken on the next iteration of the findlinks command cronjob. And anyways, I doubt that there will that many cross-region links that are really breaking by removing the region. Most of the links will be within one region, and then it doesn't matter if the link breaks and the then link is deleted a few seconds afterwards.