Skip to content

Commit

Permalink
Bug 672875 - Fix DB race conditions in actioncounter and contentflagg…
Browse files Browse the repository at this point in the history
…ing apps

* MySQL UNIQUE constraint doesn't work the way I thought it did for NULL
  values in columns. So, moved to calculating a unique hash in Python
  and store that as a unique column

* actioncounters and contentflagging converted to use South migrations

* South schema migrations to add unique_hash column and remove unused
  session_key column.

* South data migration to calculate unique_hash for all existing rows.

* Removed previously implemented bandaid fix that would delete multiple
  columns when integrity issue encountered.

* Cron job for actioncounters now deletes counters over two months old
  for anonymous users as a means of garbage collection.
  • Loading branch information
lmorchard committed Aug 31, 2011
1 parent bea56b4 commit 7eba73f
Show file tree
Hide file tree
Showing 17 changed files with 745 additions and 119 deletions.
12 changes: 12 additions & 0 deletions apps/actioncounters/cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import cronjobs

# TODO: Figure out a way to do this per-class? Would need to break up some of the SQL calls.
ACTIONCOUNTERS_ANON_GC_WINDOW = getattr(settings, "ACTIONCOUNTERS_ANON_GC_WINDOW", "2 MONTH")
ACTIONCOUNTERS_RECENT_COUNT_WINDOW = getattr(settings, "ACTIONCOUNTERS_RECENT_COUNT_WINDOW", "14 DAY")

@cronjobs.register
Expand All @@ -27,6 +28,17 @@ def get_update(ct_pk, obj_pk):

cursor = connection.cursor()

# Garbage collect any counters for anonymous users over a certain age.
cursor.execute("""
DELETE
FROM actioncounters_actioncounterunique
WHERE
user_id IS NULL AND
modified < date_sub(now(), INTERVAL %(interval)s)
""" % dict(
interval=ACTIONCOUNTERS_ANON_GC_WINDOW
))

# Any counters too old for the window should be set to 0
cursor.execute("""
SELECT content_type_id, object_pk, name
Expand Down
116 changes: 116 additions & 0 deletions apps/actioncounters/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# encoding: utf-8
import datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models

class Migration(SchemaMigration):

def forwards(self, orm):

# Adding model 'TestModel'
db.create_table('actioncounters_testmodel', (
('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
('title', self.gf('django.db.models.fields.CharField')(unique=True, max_length=255)),
('views_total', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('views_recent', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('boogs_total', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('boogs_recent', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('likes_total', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('likes_recent', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('frobs_total', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
('frobs_recent', self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True, blank=True)),
))
db.send_create_signal('actioncounters', ['TestModel'])

# Adding model 'ActionCounterUnique'
db.create_table('actioncounters_actioncounterunique', (
('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
('content_type', self.gf('django.db.models.fields.related.ForeignKey')(related_name='content_type_set_for_actioncounterunique', to=orm['contenttypes.ContentType'])),
('object_pk', self.gf('django.db.models.fields.CharField')(max_length=32)),
('name', self.gf('django.db.models.fields.CharField')(max_length=64, db_index=True)),
('total', self.gf('django.db.models.fields.IntegerField')()),
('ip', self.gf('django.db.models.fields.CharField')(db_index=True, max_length=40, null=True, blank=True)),
('session_key', self.gf('django.db.models.fields.CharField')(db_index=True, max_length=40, null=True, blank=True)),
('user_agent', self.gf('django.db.models.fields.CharField')(db_index=True, max_length=128, null=True, blank=True)),
('user', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'], null=True, blank=True)),
('modified', self.gf('django.db.models.fields.DateTimeField')(auto_now=True, blank=True)),
))
db.send_create_signal('actioncounters', ['ActionCounterUnique'])


def backwards(self, orm):

# Deleting model 'TestModel'
db.delete_table('actioncounters_testmodel')

# Deleting model 'ActionCounterUnique'
db.delete_table('actioncounters_actioncounterunique')


models = {
'actioncounters.actioncounterunique': {
'Meta': {'object_name': 'ActionCounterUnique'},
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'content_type_set_for_actioncounterunique'", 'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'ip': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '40', 'null': 'True', 'blank': 'True'}),
'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}),
'object_pk': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
'session_key': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '40', 'null': 'True', 'blank': 'True'}),
'total': ('django.db.models.fields.IntegerField', [], {}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}),
'user_agent': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '128', 'null': 'True', 'blank': 'True'})
},
'actioncounters.testmodel': {
'Meta': {'object_name': 'TestModel'},
'boogs_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'boogs_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'frobs_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'frobs_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'likes_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'likes_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'title': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'views_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'views_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'})
},
'auth.group': {
'Meta': {'object_name': 'Group'},
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
},
'auth.permission': {
'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
},
'auth.user': {
'Meta': {'object_name': 'User'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
},
'contenttypes.contenttype': {
'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
}
}

complete_apps = ['actioncounters']
92 changes: 92 additions & 0 deletions apps/actioncounters/migrations/0002_unique_hash_index.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
# encoding: utf-8
import datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models

class Migration(SchemaMigration):

def forwards(self, orm):

# Deleting field 'ActionCounterUnique.session_key'
db.delete_column('actioncounters_actioncounterunique', 'session_key')

# Adding field 'ActionCounterUnique.unique_hash'
db.add_column('actioncounters_actioncounterunique', 'unique_hash', self.gf('django.db.models.fields.CharField')(blank=True, max_length=32, null=True, db_index=True), keep_default=False)


def backwards(self, orm):

# Adding field 'ActionCounterUnique.session_key'
db.add_column('actioncounters_actioncounterunique', 'session_key', self.gf('django.db.models.fields.CharField')(blank=True, max_length=40, null=True, db_index=True), keep_default=False)

# Deleting field 'ActionCounterUnique.unique_hash'
db.delete_column('actioncounters_actioncounterunique', 'unique_hash')


models = {
'actioncounters.actioncounterunique': {
'Meta': {'object_name': 'ActionCounterUnique'},
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'content_type_set_for_actioncounterunique'", 'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'ip': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '40', 'null': 'True', 'blank': 'True'}),
'modified': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}),
'object_pk': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
'total': ('django.db.models.fields.IntegerField', [], {}),
'unique_hash': ('django.db.models.fields.CharField', [], {'default': "''", 'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}),
'user_agent': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '128', 'null': 'True', 'blank': 'True'})
},
'actioncounters.testmodel': {
'Meta': {'object_name': 'TestModel'},
'boogs_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'boogs_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'frobs_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'frobs_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'likes_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'likes_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'title': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'views_recent': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'}),
'views_total': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True', 'blank': 'True'})
},
'auth.group': {
'Meta': {'object_name': 'Group'},
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
},
'auth.permission': {
'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
},
'auth.user': {
'Meta': {'object_name': 'User'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
},
'contenttypes.contenttype': {
'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
}
}

complete_apps = ['actioncounters']
Loading

0 comments on commit 7eba73f

Please sign in to comment.