Fixup the denormalizing PR

- Refunding flights triggers recalc of totals
- Handle case where a flight is slightly overfilled
- Run denormalized calcs every 5m
- Don't bother with "error counts" in denormalized calcs.
  It will either succeed or error and alert us.

Author: davidfischer

adserver/admin.py

@@ -928,9 +928,14 @@ def refund_impressions(self, request, queryset):
         )
 
         count = 0
+        flights = set()
         for impression in queryset:
             if impression.refund():
                 count += 1
+                flights.add(impression.advertisement.flight)
+
+        for flight in flights:
+            flight.refresh_denormalized_totals()
 
         messages.add_message(
             request,

adserver/models.py

@@ -1506,6 +1506,12 @@ def refresh_denormalized_totals(self):
 
         self.total_views = aggregation["total_views"] or 0
         self.total_clicks = aggregation["total_clicks"] or 0
+
+        if self.sold_clicks > 0 and self.total_clicks > self.sold_clicks:
+            self.total_clicks = self.sold_clicks
+        if self.sold_impressions > 0 and self.total_views > self.sold_impressions:
+            self.total_views = self.sold_impressions
+
         self.save(update_fields=["total_views", "total_clicks"])
 
     def copy_niche_targeting_urls(self, other_flight):

adserver/tasks.py

@@ -939,30 +939,9 @@ def refresh_flight_denormalized_totals():
     # Only refresh active flights to avoid unnecessary work
     flights = Flight.objects.filter(live=True)
     total_flights = flights.count()
-    success_count = 0
-    error_count = 0
 
     for flight in flights:
-        try:
-            flight.refresh_denormalized_totals()
-            success_count += 1
-        except Exception as e:
-            error_count += 1
-            log.error(
-                "Failed to refresh denormalized totals for flight %s: %s",
-                flight.slug,
-                e,
-                exc_info=True,
-            )
-
-    duration = (timezone.now() - start_time).total_seconds()
-    log.info(
-        "Finished refreshing denormalized totals: %d/%d succeeded, %d failed, took %.2fs",
-        success_count,
-        total_flights,
-        error_count,
-        duration,
-    )
+        flight.refresh_denormalized_totals()
 
     # Update cache with last successful run timestamp
     cache.set(
@@ -971,14 +950,12 @@ def refresh_flight_denormalized_totals():
         timeout=None,  # Never expire
     )
 
-    # Alert if there are significant failures
-    if error_count > 0 and error_count / max(total_flights, 1) > 0.1:
-        slack_message(
-            "adserver/slack/generic-message.slack",
-            {
-                "text": f"⚠️ Flight denormalized total refresh had {error_count}/{total_flights} failures"
-            },
-        )
+    duration = (timezone.now() - start_time).total_seconds()
+    log.info(
+        "Finished refreshing denormalized totals: %d flights, took %.2fs",
+        total_flights,
+        duration,
+    )
 
 
 @app.task()

config/settings/production.py

@@ -204,7 +204,7 @@
     },
     "frequent-refresh-flight-totals": {
         "task": "adserver.tasks.refresh_flight_denormalized_totals",
-        "schedule": crontab(minute="*/10"),  # Every 10 minutes
+        "schedule": crontab(minute="*/5"),  # Every 5 minutes
     },
     # Run publisher importers daily
     "every-day-sync-publisher-data": {