Fix PG foreign key error on deleting devices #232
Conversation
|
Thinking about this some more, I'm pretty certain that the logic in the sidekiq job isn't quite right - it deletes archived if they were created more than 24 hours ago, rather than if they were archived more than 24 hours ago. (so devices which are more than 24 hours old will be deleted more or less immediately when archived) in this respect, the current bug is kinda obscuring this more fundamental one - devices are never being deleted due to the error, rather than being deleted too early. Therefore, I think i need to add a column to the devices model to store an archived_at date and populate it when it is archived then use that date in the test in the sidekiq job. For devices that are already archived, i'll set the date to 'now', so that they get a 24 hour grace period after deploy - that way we can be sure nothing will be deleted before the end of the grace period. |
|
@viktorsmari pinging you here to check briefly if you can kindly give us an input. Are we missing something? |
|
Hi! Are you looking at the smartcitizen-api/app/jobs/delete_archived_devices_job.rb Lines 9 to 10 in 2682efd Maybe that can help debugging 🤓 |
|
@viktorsmari Thanks so much for the input! Checking the logs as suggested: Looking at the sentry logs (obviously many as each device is retried), we see the 15581 retries: And finally looking at the devices in the Archive: For instance device But nothing, nowhere, for the Conclusion: I agree with @tim that the way to go would be to add a column named |
|
Is the problem only happening with one device, id 15581? I, [2023-03-22T09:33:08.599846 #1] INFO -- : deleting archived device 16135 <-- I only see this 1x in the log, so it was deleted? |
It's not on the Devices or in the DevicesArchive, so yes (?)
That's the one that appears most times in the log, but there are other occurrences. |
This fixes #214 - deleting devices failed since we moved the postprocessing data to a seperate table, as the foreign key constraint prevented the postprocessing table entry being orphaned. The solution is to make this relationship `depenedent: destroy` - when a device is removed, we remove its postprocessing info too. I've also added a test to guard against regressions here and make the behaviour we expect explicit.
This was a bug that was hidden by the existing FK error - when deleting archived devices, we check the date they were created, not archived. This has the result that devices *created* more than 24 hours ago don't have the 24 hour grace period and are deleted immediately. To fix this, we now store an archived_at date on the device, which is populated on archive, and check this in the deletion job.
timcowlishaw
force-pushed
the
bugfix/214-delete-devices-raises-fk-error
branch
from
a36429b to
69e7682
Compare
|
Hey @oscgonfer I've added an |
|
Thanks @timcowlishaw |
|
Hi! Tested in staging with success! Two devices, one with postprocessing and another with postprocessing= @timcowlishaw I would suggest to document this in Anyway, merging it! |
This fixes #214 - deleting devices failed since we moved the postprocessing data to a seperate table, as the foreign key constraint prevented the postprocessing table entry being orphaned.
The solution is to make this relationship
depenedent: destroy- when a device is removed, we remove its postprocessing info too.I've also added a test to guard against regressions here and make the behaviour we expect explicit.