一个由Django的save方法引发的bug

bug的产生

我已经很久没碰到让人比较头疼的bug了，前两天的时候碰到了一个。写个文章记录下来。 希望看过文章的同学下次碰到类似bug就不用被烦恼到。

出现问题的这段代码简化过的逻辑大概是这个样子的：

from app.models import User
from celery.task import task

... ...

# 假设我们取到的user的age和name都是None
user = User.objects.get(pk=1)
# 开启两个task来执行任务
update_user_age.delay(user)
update_user_name.delay(user)


# Celery tasks

@task
def update_user_age(user):
    """
    接受一个User实例，然后修改它的年龄
    """
    user.age = 38
    user.save()


@task
def update_user_name(user):
    """
    接受一个User实例，然后修改它的名字
    """
    user.name = 'piglei'
    user.save()

这段代码的执行结果是，user的age和name字段永远 不会都被赋上值 ，要么是有name没有age， 要么有age没有name。

怎么样？能一眼能看出来问题在什么地方吗？如果能看出来的话，可以直接跳到最后一段啦。 不过当时笔者碰到的代码比这个要复杂不少，有一些task是通过django的signals触发 调用的，所以给排查产生了很大的麻烦，花了一些功夫才找到问题。

对于那些不能看出来问题的同学，且往下看。

from app.models import User

user = User.objects.get(pk=1)
user.age = 38
user.save()

UPDATE "user_table_name" set age = 38, name = 'name_value' ... ... WHERE "user_table_name".id = USER_PK;

解决办法

知道了这个问题的原因之后，要解决起来其实也是比较简单的。最简单的解决办法只要让celery处理的时候不要调用save方法即可。

我使用了一个django snippet:

def update(instance, **kwargs):
    "Atomically update instance, setting field/value pairs from kwargs"
    # fields that use auto_now=True should be updated corrected, too!
    for field in instance._meta.fields:
        if hasattr(field, 'auto_now') and field.auto_now and field.name not in kwargs:
            kwargs[field.name] = field.pre_save(instance, False)

    rows_affected = instance.__class__._default_manager.filter(pk=instance.pk).update(**kwargs)

    # apply the updated args to the instance to mimic the change
    # note that these might slightly differ from the true database values
    # as the DB could have been updated by another thread. callers should
    # retrieve a new copy of the object if up-to-date values are required
    for k,v in kwargs.iteritems():
        if isinstance(v, ExpressionNode):
            v = resolve_expression_node(instance, v)
        setattr(instance, k, v)

    # If you use an ORM cache, make sure to invalidate the instance!
    #cache.set(djangocache.get_cache_key(instance=instance), None, 5)
    return rows_affected

这个update方法主要是实现了原子级别的update。用法很简单，把你需要修改的字段和内容作为**kwargs传进去调用即可。这样就避免了使用save时产生的一些不必要的修改。

把前面例子中的task里面的save方法改写成update，问题就能得到解决了。

update(user, age=age)
update(user, name=name)

Django新版本对这样的情景提供的支持

其实在Django 1.5以后的版本，已经对这样的情景提供了支持：

https://docs.djangoproject.com/en/1.6/ref/models/instances/#specifying-which-fields-to-save

在save方法里面新增加了一个update_fields参数。这样就可以只修改特定字段了：

user.name = name
user.save(update_fields=['name'])

这样便有效避免了并行save产生的数据冲突。不过笔者目前的生产环境还是1.3，所以还是暂时用之前的update方法顶着了。