Lock affected rows when assigning local IDs
authorMitja Nikolaus <mitja@fairphone.com>
Tue, 13 Nov 2018 16:12:51 +0000 (17:12 +0100)
committerMitja Nikolaus <mitja@fairphone.com>
Fri, 7 Dec 2018 12:54:10 +0000 (12:54 +0000)
Lock the targeted device when calculating the next heartbeat and crash
report local IDs. Lock the targeted crash report when calculating the
next log file local ID.

This change prevents race conditions that have occured previously.

Issue: HIC-251
Change-Id: I80a8498849e20769b0ab36afe68e6c20b3108c6d

crashreports/models.py
crashreports/tests/test_rest_api_crashreports.py
crashreports/tests/test_rest_api_heartbeats.py
crashreports/tests/test_rest_api_logfiles.py

index f76c894..a405620 100644 (file)
@@ -46,17 +46,19 @@ class Device(models.Model):
     @transaction.atomic
     def get_crashreport_key(self):
         """Get the next key for a crashreport and update the ID-counter."""
-        ret = self.next_per_crashreport_key
-        self.next_per_crashreport_key = self.next_per_crashreport_key + 1
-        self.save()
+        device = Device.objects.select_for_update().get(id=self.id)
+        ret = device.next_per_crashreport_key
+        device.next_per_crashreport_key += 1
+        device.save()
         return ret
 
     @transaction.atomic
     def get_heartbeat_key(self):
         """Get the next key for a heartbeat and update the ID-counter."""
-        ret = self.next_per_heartbeat_key
-        self.next_per_heartbeat_key = self.next_per_heartbeat_key + 1
-        self.save()
+        device = Device.objects.select_for_update().get(id=self.id)
+        ret = device.next_per_heartbeat_key
+        device.next_per_heartbeat_key += 1
+        device.save()
         return ret
 
 
@@ -118,9 +120,10 @@ class Crashreport(models.Model):
     @transaction.atomic
     def get_logfile_key(self):
         """Get the next key for a log file and update the ID-counter."""
-        ret = self.next_logfile_key
-        self.next_logfile_key = self.next_logfile_key + 1
-        self.save()
+        crashreport = Crashreport.objects.select_for_update().get(id=self.id)
+        ret = crashreport.next_logfile_key
+        crashreport.next_logfile_key += 1
+        crashreport.save()
         return ret
 
     def save(
index b09ca1d..11b8e8e 100644 (file)
@@ -1,5 +1,4 @@
 """Tests for the crashreports REST API."""
-import unittest
 from datetime import timedelta
 
 from django.db import connection
@@ -54,7 +53,6 @@ class CrashreportsTestCase(HeartbeatsTestCase):
         pass
 
 
-@unittest.skip("Fails because of race condition when assigning local IDs")
 class CrashreportRaceConditionsTestCase(RaceConditionsTestCase):
     """Test cases for crashreport race conditions."""
 
index d2c3e1a..ac2e03e 100644 (file)
@@ -1,6 +1,5 @@
 """Tests for the heartbeats REST API."""
 from datetime import timedelta, datetime
-import unittest
 
 import pytz
 from django.db import connection
@@ -322,7 +321,6 @@ class HeartbeatsTestCase(HiccupCrashreportsAPITestCase):
         self.assertEqual(response.data["date"], str(data["date"].date()))
 
 
-@unittest.skip("Fails because of race condition when assigning local IDs")
 class HeartBeatRaceConditionsTestCase(RaceConditionsTestCase):
     """Test cases for heartbeat race conditions."""
 
index b0aeb34..1cf2e55 100644 (file)
@@ -3,7 +3,6 @@
 import os
 import shutil
 import tempfile
-import unittest
 import zipfile
 
 from django.conf import settings
@@ -153,7 +152,6 @@ class LogfileUploadTest(HiccupCrashreportsAPITestCase):
         shutil.rmtree(settings.MEDIA_ROOT)
 
 
-@unittest.skip("Fails because of race condition when assigning local IDs")
 class LogfileRaceConditionsTestCase(RaceConditionsTestCase):
     """Test cases for logfile race conditions."""