"consistency" (C From 'acid') When Using Savecounters()

Yii 1.1 General Discussion
1

Edit: the below assumption is wrong - consistency is handled ok in saveCounters() code. See subsequent posts…

Hi,

I noticed this handy CActiveRecord method saveCounters() .

While useful, AFAIK it doesn’t handle the ‘consistency’ in ACID: Two competing processes ‘read’ the same row/record. One saveCounters() and the other one does the same - effectively overwriting the first writing process data, and there goes data integrity out the window (if data integrity matters - depending on your use case).

I didn’t notice any sort of solution to this in the code, for example some sort of locking, isolating each process. [size=“2”]Please correct me if I’m wrong in this observation.[/size]

[size="2"]One way to implement this would be with "SELECT… FOR UPDATE" but this is probably non portable between several DBs (MySQL, PostGresql, etc…) and depending on DB server configuration, which even less reduces its portability.[/size]

[size=“2”]My personal tendency is to take a different approach and use an extension I wrote a while ago that includes an optimistic locking implementation. With it, I can make some fixed number of attempts to ‘safely save’ the counters (not using saveCounters()) while avoiding race conditions and ‘retry’ if anyone updated the record before.[/size]

[size="2"]All in all - any comments on the above? Better way to achieve this?[/size]

Thanks!

2

Hi

I looked at the code and it looks partially ok: the database will be ok, but not the php values…


public function saveCounters($counters)

{

    Yii::trace(get_class($this).'.saveCounters()','system.db.ar.CActiveRecord');

    $builder=$this->getCommandBuilder();

    $table=$this->getTableSchema();

    $criteria=$builder->createPkCriteria($table,$this->getOldPrimaryKey());

    $command=$builder->createUpdateCounterCommand($this->getTableSchema(),$counters,$criteria);

    if($command->execute())

    {

        foreach($counters as $name=>$value)

            $this->$name=$this->$name+$value;

        return true;

    }

    else

        return false;

}

The createUpdateCounterCommand will do an ‘UPDATE’ command in the database, leaving it to the database to compute the value.

However, if execution is ok, the model’s values are updated by computing the sum in PHP, not by refreshing the data from the database.

I guess that if you modify ‘saveCounters’ in the model’s code to include a ‘refresh’ that this is fixed.


public function saveCounters($counters)

{

    if(parent::saveCounters($counters)) {

        $this->refresh();

        return true;

    } else {

        return false;

    }

}

The side effect is the the refresh will retrieve all the fields, not just the counters.

3

EDIT:

Indeed, the ‘UPDATE’ constructs a statement that updates the counter value based on the existing value. This ‘update’ statement does not include actual updated values but just an increment/decrement directive.[size=2] I think this might be the answer to the race condition, indeed.[/size]

[size=2]The PHP calculation is what I call a bug [/size][size=2] . I’ll open one to discuss with the dev team. Easiest way to resolve would be to do a refresh(), as you suggested.[/size]

4

Bug opened.