0

Here's my model:

class MyModel(BaseModel):
    no = models.PositiveIntegerField()  # Human-friendly no -> counter that starts from 1 for every type
    type = models.ForeignKey(MyModelType)

I set the value for the no field in model's pre_save signal:

@receiver(pre_save, sender=MyModel)
def mymodel_pre_save(sender, instance, *args, **kwargs):
    if not instance._state.adding:
        return

    obj_count = MyModel.objects.filter(type=instance.type).count()
    instance.no = obj_count + 1 

The bug with this code is that it produces different objects having same no number. Probably it happens when multiple users create objects at the same time.

What's the best way to fix it? Would moving assignment to .save() method of the model suffice in high-load environment?

Solution

Eventually, I've implemented @moooeeeep's solution but moved the logic from post_save to model's save method:

class MyModel(BaseModel):
    ...

    def save(self, *args, **kwargs):
        adding = self.pk is None

        with transaction.atomic():
            super().save(*args, **kwargs)

            if adding:
                # Compute and set `.no`
                obj_count = MyModel.objects.filter(type=self.type, id__lte=self.id).count()
                self.no = obj_count
                self.save(update_fields=['no'])
AlexSec
  • 13
  • 4
  • Are you trying to do something like this? https://stackoverflow.com/q/21128899/1025391 – moooeeeep Dec 12 '17 at 08:57
  • @moooeeeep, no. A simple `AutoField` field won't do. I need the counter to count records that hold same `type` value. – AlexSec Dec 12 '17 at 12:53

2 Answers2

0

You can use select_for_update() method to get instance object for db currence traction probelm.but In your high-load environment,the select_for_update method will lead to waiting.

Solution 1:I suggest you move this count change code form pre_save to post_save,if so,you don't need to do plus one,just put the new count to no field.

Solution 2:don't store the type count no in database,just get it from query or cache it in redisDB.

My English is very poor, Sorry.

Hayden
  • 449
  • 4
  • 13
  • 1. I fear that if something bad happens in the `post_save` signal then there would be instances with empty values in the `no` field. 2. I think, calculating the counter dynamically might lead to inconsistent situations when old instances got updated counter values due to removing some other instances from the DB, etc.. – AlexSec Dec 12 '17 at 12:48
  • you shuold not use post_save,you should use backgroud offline task to update it in db or cachedb if the value of count must not be very consistent.Your domain context need to be more clear. – Hayden Dec 12 '17 at 13:52
0

I could imagine that your problem is caused by a race condition:

  • One request asks for the count,
  • another request asks for the count,
  • one request inserts a new item with count+1,
  • another request inserts a new item with the same count+1.

The most straightforward way I can imagine would be, that you don't set the count of the item before insertion, but after. You then compute the count for objects of that type by counting the number of items with the appropriate type where the id is smaller than or equal to the id of the respective object. Then you update that value in a post_save() handler.

Note that your count field might still go wrong, when items are removed after you counted them. When you want to allow item removal, you might need to update all counts with an id greater than the id you're going to remove.

In conclusion, it might be a bad idea to assign the count from the Django side of things altogether. Consider to use database functionality to assign the count as default value computed from the (broadly speaking) max(count) where the type equals the type of your newly inserted item or zero plus one. And don't worry about items that are going to be removed. If you want an accurate count, make a counting query when you need it.

moooeeeep
  • 31,622
  • 22
  • 98
  • 187