Skip to content

Commit

Permalink
OSIDB-3579: Implement alert versioning to reduce database locks (#856)
Browse files Browse the repository at this point in the history
- Added new field `last_validated_dt` to the `AlertMixin` and
`created_dt` to the `Alert` Model.
- Remove the `alerts.all.delete()` method call to prevent database locks
- Modify the `AlertSerializer` to filter out alerts with a `created_dt`
older than the `last_validated_dt` of the object they are related
- Created a `stale_alert_cleanup` celery task to run periodically

Closes OSIDB-3579
  • Loading branch information
MrMarble authored Dec 12, 2024
2 parents 7bc59d2 + a7ab0bb commit 1270689
Show file tree
Hide file tree
Showing 8 changed files with 549 additions and 60 deletions.
8 changes: 7 additions & 1 deletion config/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,13 @@
("*", {"queue": "fast"}), # default other tasks go to 'fast'
],
)
CELERY_BEAT_SCHEDULE = {}
CELERY_BEAT_SCHEDULE = {
"stale_alert_cleanup": {
"task": "osidb.tasks.stale_alert_cleanup",
# Run every hour
"schedule": crontab(minute=0),
}
}

# There are multiple possible memory leaks in Celery. To work around these issues it is
# recommended to restart Celery worker children processes after set number of tasks.
Expand Down
342 changes: 342 additions & 0 deletions osidb/migrations/0177_remove_affect_insert_insert_and_more.py

Large diffs are not rendered by default.

23 changes: 18 additions & 5 deletions osidb/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,8 @@ class AlertType(models.TextChoices):

objects = AlertManager()

created_dt = models.DateTimeField(blank=True, default=timezone.now)

def _validate_acl_identical_to_parent(self, **kwargs):
if (
self.acl_read != self.content_object.acl_read
Expand Down Expand Up @@ -687,6 +689,15 @@ class AlertMixin(ValidateMixin):

alerts = GenericRelation(Alert)

last_validated_dt = models.DateTimeField(blank=True, default=timezone.now)

@property
def valid_alerts(self):
"""
Get all alerts that are valid
"""
return self.alerts.filter(created_dt__gte=self.last_validated_dt)

def alert(
self,
name,
Expand Down Expand Up @@ -761,6 +772,7 @@ def alert(
resolution_steps=resolution_steps,
acl_read=acl_read,
acl_write=acl_write,
created_dt=timezone.now(),
).save()
except IntegrityError:
# alerts of the same name, object_id and age have the same meaning
Expand All @@ -785,10 +797,6 @@ def validate(self, raise_validation_error=True, dry_run=False):
# exclude meta attributes
self.full_clean(exclude=["meta_attr"])

# clean all alerts before a new validation
if not dry_run:
self.alerts.all().delete()

# custom validations
for validation_name in [
item for item in dir(self) if item.startswith("_validate_")
Expand All @@ -813,9 +821,14 @@ def save(self, *args, **kwargs):
"""
Save with validate call parametrized by raise_validation_error
"""

dry_run = kwargs.pop("no_alerts", False)
if not dry_run:
self.last_validated_dt = timezone.now()

self.validate(
raise_validation_error=kwargs.pop("raise_validation_error", True),
dry_run=kwargs.pop("no_alerts", False),
dry_run=dry_run,
)
# here we have to skip ValidateMixin level save as otherwise
# it would run validate again and without proper arguments
Expand Down
10 changes: 9 additions & 1 deletion osidb/serializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,13 +548,21 @@ def get_parent_model(self, obj):
class AlertMixinSerializer(serializers.ModelSerializer):
"""Serializes the alerts in models that implement AlertMixin."""

alerts = AlertSerializer(many=True, read_only=True)
alerts = serializers.SerializerMethodField()

class Meta:
model = AlertMixin
abstract = True
fields = ["alerts"]

@extend_schema_field(AlertSerializer(many=True))
def get_alerts(self, instance):
query_set = Alert.objects.filter(
object_id=instance.uuid, created_dt__gte=instance.last_validated_dt
)
serializer = AlertSerializer(query_set, many=True, read_only=True)
return serializer.data


class AuditSerializer(serializers.ModelSerializer):

Expand Down
61 changes: 61 additions & 0 deletions osidb/tasks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
from celery.utils.log import get_task_logger
from django.conf import settings
from django.contrib.contenttypes.models import ContentType
from django.db.models import OuterRef, Q, Subquery
from django.db.models.functions import Cast

from config.celery import app
from osidb.core import set_user_acls
from osidb.mixins import Alert

logger = get_task_logger(__name__)


@app.task
def stale_alert_cleanup():
"""Delete stale alerts from the database.
On every run, this collector will check if the alert is still valid by comparing
the creation time of the alert with the validation time of the Model.
If the creation time of the alert is older than the validation time of the Model,
the alert is considered stale and will be deleted.
"""
set_user_acls(settings.ALL_GROUPS)

content_types = ContentType.objects.filter(
id__in=Alert.objects.values_list("content_type", flat=True).distinct()
)

logger.info(f"Searching for stale alerts in {content_types.count()} content types")

query = Q()

for content_type in content_types:
model_class = content_type.model_class()

subquery = Subquery(
model_class.objects.filter(
pk=Cast(OuterRef("object_id"), output_field=model_class._meta.pk)
)
.order_by("last_validated_dt")
.values("last_validated_dt")[:1]
)

query |= Q(
content_type=content_type,
created_dt__lt=subquery,
)

filtered_queryset = Alert.objects.filter(query)

if filtered_queryset.count() == 0:
return "No Stale Alerts Found"

logger.info(f"Found {filtered_queryset.count()} stale alerts")

deleted_alerts_count = filtered_queryset.delete()[0]

logger.info(f"Deleted {deleted_alerts_count} stale alerts")

return f"Deleted {deleted_alerts_count} Stale Alerts"
4 changes: 2 additions & 2 deletions osidb/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ def test_alert_deletion(self):

# validate() is called inside save()
m.save()
assert m.alerts.count() == 1
alert = m.alerts.first()
assert m.valid_alerts.count() == 1
alert = m.valid_alerts.first()
assert alert.name == "new_alert"
assert alert.alert_type == Alert.AlertType.WARNING
assert alert.description == "This is a new alert."
Expand Down
Loading

0 comments on commit 1270689

Please sign in to comment.