From b186d0ecb634f7fad4c0fa28d2bacf87b4001e88 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Tue, 10 Sep 2013 13:43:37 -0700 Subject: [PATCH 01/34] Initial Django node backend --- src/sentry/app.py | 2 ++ src/sentry/conf/server.py | 9 ++++- src/sentry/nodestore/__init__.py | 9 +++++ src/sentry/nodestore/base.py | 27 ++++++++++++++ src/sentry/nodestore/django/__init__.py | 9 +++++ src/sentry/nodestore/django/backend.py | 36 +++++++++++++++++++ src/sentry/nodestore/django/models.py | 27 ++++++++++++++ .../nodestore/migrations/0001_initial.py | 34 ++++++++++++++++++ src/sentry/nodestore/migrations/__init__.py | 0 src/sentry/nodestore/models.py | 13 +++++++ 10 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 src/sentry/nodestore/__init__.py create mode 100644 src/sentry/nodestore/base.py create mode 100644 src/sentry/nodestore/django/__init__.py create mode 100644 src/sentry/nodestore/django/backend.py create mode 100644 src/sentry/nodestore/django/models.py create mode 100644 src/sentry/nodestore/migrations/0001_initial.py create mode 100644 src/sentry/nodestore/migrations/__init__.py create mode 100644 src/sentry/nodestore/models.py diff --git a/src/sentry/app.py b/src/sentry/app.py index 28e08948455d9e..ea77096135d5da 100644 --- a/src/sentry/app.py +++ b/src/sentry/app.py @@ -22,4 +22,6 @@ def get_instance(path, options): buffer = get_instance(settings.SENTRY_BUFFER, settings.SENTRY_BUFFER_OPTIONS) quotas = get_instance(settings.SENTRY_QUOTAS, settings.SENTRY_QUOTA_OPTIONS) +nodestore = get_instance( + settings.SENTRY_NODESTORE, settings.SENTRY_NODESTORE_OPTIONS) env = State() diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 45d6845e678b7f..c46818ce6fac75 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -150,6 +150,7 @@ 'kombu.transport.django', 'raven.contrib.django.raven_compat', 'sentry', + 'sentry.nodestore', 'sentry.plugins.sentry_interface_types', 'sentry.plugins.sentry_mail', 'sentry.plugins.sentry_urls', @@ -506,12 +507,18 @@ # Redis connection information (see Nydus documentation) SENTRY_REDIS_OPTIONS = {} -# Buffer backend to use +# Buffer backend SENTRY_BUFFER = 'sentry.buffer.Buffer' SENTRY_BUFFER_OPTIONS = {} +# Quota backend SENTRY_QUOTAS = 'sentry.quotas.Quota' SENTRY_QUOTA_OPTIONS = {} + +# Node storage backend +SENTRY_NODESTORE = 'sentry.nodestore.django.DjangoNodeStorage' +SENTRY_NODESTORE_OPTIONS = {} + # The default value for project-level quotas SENTRY_DEFAULT_MAX_EVENTS_PER_MINUTE = '90%' # The maximum number of events per minute the system should accept. diff --git a/src/sentry/nodestore/__init__.py b/src/sentry/nodestore/__init__.py new file mode 100644 index 00000000000000..a28d6b91781b34 --- /dev/null +++ b/src/sentry/nodestore/__init__.py @@ -0,0 +1,9 @@ +""" +sentry.nodestore +~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py new file mode 100644 index 00000000000000..5ed647c6d2bb78 --- /dev/null +++ b/src/sentry/nodestore/base.py @@ -0,0 +1,27 @@ +""" +sentry.nodestore.base +~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + + +class NodeStorage(object): + def get(self, src): + raise NotImplementedError + + def get_multi(self, src_list): + return dict( + (src, self.get(src)) + for src in src_list + ) + + def set(self, src, data, timestamp=None): + raise NotImplementedError + + def set_multi(self, values): + for v in values: + self.set(**v) diff --git a/src/sentry/nodestore/django/__init__.py b/src/sentry/nodestore/django/__init__.py new file mode 100644 index 00000000000000..d03f3a57f8c4b7 --- /dev/null +++ b/src/sentry/nodestore/django/__init__.py @@ -0,0 +1,9 @@ +""" +sentry.nodestore.django +~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from .backend import DjangoNodeStorage # NOQA diff --git a/src/sentry/nodestore/django/backend.py b/src/sentry/nodestore/django/backend.py new file mode 100644 index 00000000000000..4b81859d7c636f --- /dev/null +++ b/src/sentry/nodestore/django/backend.py @@ -0,0 +1,36 @@ +""" +sentry.nodestore.django.backend +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + +from django.utils import timezone + +from sentry.db.models import create_or_update +from sentry.nodestore.base import NodeStorage + +from .models import Node + + +class DjangoNodeStorage(NodeStorage): + def get(self, src): + return Node.objects.get(src=src) + + def get_multi(self, src_list): + return Node.objects.get(src__in=src_list) + + def set(self, src, data, timestamp=None): + create_or_update( + Node, + src=src, + data=data, + timestamp=timestamp or timezone.now() + ) + + def set_multi(self, values): + for v in values: + self.set(**v) diff --git a/src/sentry/nodestore/django/models.py b/src/sentry/nodestore/django/models.py new file mode 100644 index 00000000000000..77d3277fdb22d4 --- /dev/null +++ b/src/sentry/nodestore/django/models.py @@ -0,0 +1,27 @@ +""" +sentry.nodestore.django.models +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + +from django.db import models +from django.utils import timezone + +from sentry.db.models import ( + BaseModel, GzippedDictField, sane_repr) + + +class Node(BaseModel): + # TODO: should we just UUID this and use claims? + src = models.AutoField(primary_key=True) + data = GzippedDictField() + timestamp = models.DateTimeField(default=timezone.now) + + __repr__ = sane_repr('src', 'timestamp') + + class Meta: + app_label = 'nodestore' diff --git a/src/sentry/nodestore/migrations/0001_initial.py b/src/sentry/nodestore/migrations/0001_initial.py new file mode 100644 index 00000000000000..c77fe814301a3e --- /dev/null +++ b/src/sentry/nodestore/migrations/0001_initial.py @@ -0,0 +1,34 @@ +# -*- coding: 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 'Node' + db.create_table(u'nodestore_node', ( + ('src', self.gf('django.db.models.fields.IntegerField')(primary_key=True)), + ('data', self.gf('django.db.models.fields.TextField')()), + ('timestamp', self.gf('django.db.models.fields.DateTimeField')(default=datetime.datetime.now)), + )) + db.send_create_signal('nodestore', ['Node']) + + + def backwards(self, orm): + # Deleting model 'Node' + db.delete_table(u'nodestore_node') + + + models = { + 'nodestore.node': { + 'Meta': {'object_name': 'Node'}, + 'data': ('django.db.models.fields.TextField', [], {}), + 'src': ('django.db.models.fields.IntegerField', [], {'primary_key': 'True'}), + 'timestamp': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}) + } + } + + complete_apps = ['nodestore'] diff --git a/src/sentry/nodestore/migrations/__init__.py b/src/sentry/nodestore/migrations/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/src/sentry/nodestore/models.py b/src/sentry/nodestore/models.py new file mode 100644 index 00000000000000..8d9a8e502f5f65 --- /dev/null +++ b/src/sentry/nodestore/models.py @@ -0,0 +1,13 @@ +""" +sentry.nodestore.models +~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + +# HACK(dcramer): Django doesn't play well with our naming schemes, and we prefer +# our methods ways over Django's limited scoping +from .django.models import * # NOQA From 91bbaa053bbe64c2113efa33918671675fbfdc85 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 11:41:49 +0900 Subject: [PATCH 02/34] Switch Node pk to UUID --- setup.py | 1 + src/sentry/nodestore/django/models.py | 5 +++-- src/sentry/nodestore/migrations/0001_initial.py | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/setup.py b/setup.py index 47b77f84f054aa..19b844aea56d69 100755 --- a/setup.py +++ b/setup.py @@ -72,6 +72,7 @@ 'django-picklefield>=0.3.0,<0.4.0', 'django-static-compiler>=0.3.0,<0.4.0', 'django-templatetag-sugar>=0.1.0,<0.2.0', + 'django-uuidfield==0.4.0', 'gunicorn>=0.17.2,<0.18.0', 'logan>=0.5.7,<0.6.0', 'nydus>=0.10.0,<0.11.0', diff --git a/src/sentry/nodestore/django/models.py b/src/sentry/nodestore/django/models.py index 77d3277fdb22d4..862ed981528aec 100644 --- a/src/sentry/nodestore/django/models.py +++ b/src/sentry/nodestore/django/models.py @@ -8,6 +8,8 @@ from __future__ import absolute_import +from uuidfield import UUIDField + from django.db import models from django.utils import timezone @@ -16,8 +18,7 @@ class Node(BaseModel): - # TODO: should we just UUID this and use claims? - src = models.AutoField(primary_key=True) + id = UUIDField(auto=True, primary_key=True) data = GzippedDictField() timestamp = models.DateTimeField(default=timezone.now) diff --git a/src/sentry/nodestore/migrations/0001_initial.py b/src/sentry/nodestore/migrations/0001_initial.py index c77fe814301a3e..5c0254fefee90e 100644 --- a/src/sentry/nodestore/migrations/0001_initial.py +++ b/src/sentry/nodestore/migrations/0001_initial.py @@ -10,7 +10,7 @@ class Migration(SchemaMigration): def forwards(self, orm): # Adding model 'Node' db.create_table(u'nodestore_node', ( - ('src', self.gf('django.db.models.fields.IntegerField')(primary_key=True)), + ('id', self.gf('uuidfield.fields.UUIDField')(unique=True, max_length=32, primary_key=True)), ('data', self.gf('django.db.models.fields.TextField')()), ('timestamp', self.gf('django.db.models.fields.DateTimeField')(default=datetime.datetime.now)), )) @@ -26,9 +26,9 @@ def backwards(self, orm): 'nodestore.node': { 'Meta': {'object_name': 'Node'}, 'data': ('django.db.models.fields.TextField', [], {}), - 'src': ('django.db.models.fields.IntegerField', [], {'primary_key': 'True'}), + 'id': ('uuidfield.fields.UUIDField', [], {'unique': 'True', 'max_length': '32', 'primary_key': 'True'}), 'timestamp': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}) } } - complete_apps = ['nodestore'] + complete_apps = ['nodestore'] \ No newline at end of file From 57b6643cc6ea75f04af375ef459dba3073bbd15f Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 13:18:40 +0900 Subject: [PATCH 03/34] Basic tests and working django implementation --- src/sentry/nodestore/base.py | 10 ++--- src/sentry/nodestore/django/backend.py | 20 ++++++--- tests/sentry/nodestore/__init__.py | 0 tests/sentry/nodestore/django/__init__.py | 0 .../nodestore/django/backend/__init__.py | 0 .../sentry/nodestore/django/backend/tests.py | 44 +++++++++++++++++++ 6 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 tests/sentry/nodestore/__init__.py create mode 100644 tests/sentry/nodestore/django/__init__.py create mode 100644 tests/sentry/nodestore/django/backend/__init__.py create mode 100644 tests/sentry/nodestore/django/backend/tests.py diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 5ed647c6d2bb78..0ba971a1c4918d 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -10,16 +10,16 @@ class NodeStorage(object): - def get(self, src): + def get(self, id): raise NotImplementedError - def get_multi(self, src_list): + def get_multi(self, id_list): return dict( - (src, self.get(src)) - for src in src_list + (id, self.get(id)) + for id in id_list ) - def set(self, src, data, timestamp=None): + def set(self, id, data, timestamp=None): raise NotImplementedError def set_multi(self, values): diff --git a/src/sentry/nodestore/django/backend.py b/src/sentry/nodestore/django/backend.py index 4b81859d7c636f..259e42ba730e2a 100644 --- a/src/sentry/nodestore/django/backend.py +++ b/src/sentry/nodestore/django/backend.py @@ -17,16 +17,22 @@ class DjangoNodeStorage(NodeStorage): - def get(self, src): - return Node.objects.get(src=src) - - def get_multi(self, src_list): - return Node.objects.get(src__in=src_list) + def get(self, id): + try: + return Node.objects.get(id=id).data + except Node.DoesNotExist: + return None + + def get_multi(self, id_list): + return dict( + (n.id, n.data) + for n in Node.objects.filter(id__in=id_list) + ) - def set(self, src, data, timestamp=None): + def set(self, id, data, timestamp=None): create_or_update( Node, - src=src, + id=id, data=data, timestamp=timestamp or timezone.now() ) diff --git a/tests/sentry/nodestore/__init__.py b/tests/sentry/nodestore/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/django/__init__.py b/tests/sentry/nodestore/django/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/django/backend/__init__.py b/tests/sentry/nodestore/django/backend/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/django/backend/tests.py b/tests/sentry/nodestore/django/backend/tests.py new file mode 100644 index 00000000000000..6c16e35f6a33e9 --- /dev/null +++ b/tests/sentry/nodestore/django/backend/tests.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- + +from __future__ import absolute_import + +from sentry.nodestore.django.models import Node +from sentry.nodestore.django.backend import DjangoNodeStorage +from sentry.testutils import TestCase + + +class DjangoNodeStorageTest(TestCase): + def setUp(self): + self.ns = DjangoNodeStorage() + + def test_get(self): + node = Node.objects.create( + id='d2502ebbd7df41ceba8d3275595cac33', + data={ + 'foo': 'bar', + } + ) + + result = self.ns.get('d2502ebbd7df41ceba8d3275595cac33') + assert result == node.data + + def test_get_multi(self): + nodes = [ + Node.objects.create( + id='d2502ebbd7df41ceba8d3275595cac33', + data={ + 'foo': 'bar', + } + ), + Node.objects.create( + id='5394aa025b8e401ca6bc3ddee3130edc', + data={ + 'foo': 'baz', + } + ), + ] + + result = self.ns.get_multi([ + 'd2502ebbd7df41ceba8d3275595cac33', '5394aa025b8e401ca6bc3ddee3130edc' + ]) + assert result == dict((n.id, n.data) for n in nodes) From e17fa3d01539d8603ba7bcd58ff52d93087a28a6 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 13:28:02 +0900 Subject: [PATCH 04/34] remove invalid src field from repr --- src/sentry/nodestore/django/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/nodestore/django/models.py b/src/sentry/nodestore/django/models.py index 862ed981528aec..3ce6dd1bcfbc8a 100644 --- a/src/sentry/nodestore/django/models.py +++ b/src/sentry/nodestore/django/models.py @@ -22,7 +22,7 @@ class Node(BaseModel): data = GzippedDictField() timestamp = models.DateTimeField(default=timezone.now) - __repr__ = sane_repr('src', 'timestamp') + __repr__ = sane_repr('timestamp') class Meta: app_label = 'nodestore' From 079e79a1f8261a71f4d3880477598ebd294ddabc Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 13:46:53 +0900 Subject: [PATCH 05/34] Initial work on node field --- src/sentry/db/models/fields/node.py | 104 ++++++++++++++++++++++++++++ src/sentry/models.py | 16 ++++- 2 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 src/sentry/db/models/fields/node.py diff --git a/src/sentry/db/models/fields/node.py b/src/sentry/db/models/fields/node.py new file mode 100644 index 00000000000000..bb7f63d69646ce --- /dev/null +++ b/src/sentry/db/models/fields/node.py @@ -0,0 +1,104 @@ +""" +sentry.db.models.fields.node +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + +import collections +import logging + +from django.db import models + +from sentry.utils.cache import memoize +from sentry.utils.compat import pickle +from sentry.utils.strings import decompress, compress + +from .gzippeddict import GzippedDictField + +__all__ = ('NodeField',) + +logger = logging.getLogger('sentry.errors') + + +class NodeData(collections.MutableMapping): + def __init__(self, id, data=None): + self.id = id + self._node_data = data + + @memoize + def data(self): + if self._node_data is None: + raise Exception('Must populate node data first') + return self._node_data + + def bind_node_data(self, data): + self.data = data + + def __getitem__(self, key): + return self.data[key] + + def __setitem__(self, key, value): + self.data[key] = value + + def __delitem__(self, key): + del self.data[key] + + def __iter__(self): + return iter(self.data) + + def __len__(self): + return len(self.data) + + +class NodeField(GzippedDictField): + """ + Similar to the gzippedictfield except that it stores a reference + to an external node. + """ + __metaclass__ = models.SubfieldBase + + def to_python(self, value): + if isinstance(value, basestring) and value: + try: + value = pickle.loads(decompress(value)) + except Exception, e: + logger.exception(e) + value = {} + elif not value: + value = {} + + if 'node_id' in value: + node_id = value.pop('node_id') + data = None + else: + node_id = None + data = value + + return NodeData(node_id, data) + + def get_prep_value(self, value): + if not value and self.null: + # save ourselves some storage + return None + if value.id: + result = { + 'node_id': value.id + } + else: + result = value.data + return compress(pickle.dumps(result)) + + def value_to_string(self, obj): + value = self._get_val_from_obj(obj) + return self.get_prep_value(value) + + def south_field_triple(self): + "Returns a suitable description of this field for South." + from south.modelsinspector import introspector + field_class = "django.db.models.fields.TextField" + args, kwargs = introspector(self) + return (field_class, args, kwargs) diff --git a/src/sentry/models.py b/src/sentry/models.py index 9cb6ca626b63f6..e7f93479a08ad9 100644 --- a/src/sentry/models.py +++ b/src/sentry/models.py @@ -743,7 +743,7 @@ class Meta: @memoize def interfaces(self): result = [] - for key, data in self.data.iteritems(): + for key, data in self.node_data.iteritems(): if '.' not in key: continue @@ -760,6 +760,20 @@ def interfaces(self): return SortedDict((k, v) for k, v in sorted(result, key=lambda x: x[1].get_score(), reverse=True)) + @property + def node_data(self): + assert hasattr(self, '_node_data_cache'), 'missing node data cache' + + def bind_node_data(self): + from sentry import app + + node_id = self._data.get('data_nid') + + if not node_id: + return self._data + + self._node_data_cache = app.nodestore.get(node_id) + def get_version(self): if not self.data: return From 56ae3fdcdd6f4189d3fb8cb62b299cd73845c3cf Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 14:13:10 +0900 Subject: [PATCH 06/34] Add NodeStorage.create --- src/sentry/nodestore/base.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 0ba971a1c4918d..094aafdaeb6abd 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -8,8 +8,14 @@ from __future__ import absolute_import +import uuid + class NodeStorage(object): + def create(self, data, timestamp=None): + node_id = uuid.uuid4().hex + return self.set(node_id, data, timestamp) + def get(self, id): raise NotImplementedError From a48bf5b5b4a133cf89e9406e0c20c492b213e24c Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 14:13:18 +0900 Subject: [PATCH 07/34] Swap in NodeField on Event --- src/sentry/db/models/fields/__init__.py | 1 + src/sentry/db/models/fields/node.py | 36 +++++++++++-------------- src/sentry/models.py | 11 +++----- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/src/sentry/db/models/fields/__init__.py b/src/sentry/db/models/fields/__init__.py index ed17cb9bdd93aa..38484294560958 100644 --- a/src/sentry/db/models/fields/__init__.py +++ b/src/sentry/db/models/fields/__init__.py @@ -10,3 +10,4 @@ from .bounded import * # NOQA from .gzippeddict import * # NOQA +from .node import * # NOQA diff --git a/src/sentry/db/models/fields/node.py b/src/sentry/db/models/fields/node.py index bb7f63d69646ce..62f8d56d9fadb4 100644 --- a/src/sentry/db/models/fields/node.py +++ b/src/sentry/db/models/fields/node.py @@ -29,15 +29,6 @@ def __init__(self, id, data=None): self.id = id self._node_data = data - @memoize - def data(self): - if self._node_data is None: - raise Exception('Must populate node data first') - return self._node_data - - def bind_node_data(self, data): - self.data = data - def __getitem__(self, key): return self.data[key] @@ -53,6 +44,22 @@ def __iter__(self): def __len__(self): return len(self.data) + def __repr__(self): + cls_name = type(self).__name__ + if self._node_data: + return '<%s: id=%s data=%r>' % ( + cls_name, self.id, repr(self._node_data)) + return '<%s: id=%s>' % (self.id,) + + @memoize + def data(self): + if self._node_data is None: + raise Exception('Must populate node data first') + return self._node_data + + def bind_node_data(self, data): + self.data = data + class NodeField(GzippedDictField): """ @@ -91,14 +98,3 @@ def get_prep_value(self, value): else: result = value.data return compress(pickle.dumps(result)) - - def value_to_string(self, obj): - value = self._get_val_from_obj(obj) - return self.get_prep_value(value) - - def south_field_triple(self): - "Returns a suitable description of this field for South." - from south.modelsinspector import introspector - field_class = "django.db.models.fields.TextField" - args, kwargs = introspector(self) - return (field_class, args, kwargs) diff --git a/src/sentry/models.py b/src/sentry/models.py index e7f93479a08ad9..7f097ec43a180b 100644 --- a/src/sentry/models.py +++ b/src/sentry/models.py @@ -40,7 +40,7 @@ LOG_LEVELS, MAX_CULPRIT_LENGTH, MAX_TAG_KEY_LENGTH, MAX_TAG_VALUE_LENGTH) from sentry.db.models import ( Model, GzippedDictField, BoundedIntegerField, BoundedPositiveIntegerField, - update, sane_repr) + NodeField, update, sane_repr) from sentry.manager import ( GroupManager, ProjectManager, MetaManager, InstanceMetaManager, SearchDocumentManager, BaseManager, @@ -482,7 +482,6 @@ class EventBase(Model): max_length=MAX_CULPRIT_LENGTH, blank=True, null=True, db_column='view') checksum = models.CharField(max_length=32, db_index=True) - data = GzippedDictField(blank=True, null=True) num_comments = BoundedPositiveIntegerField(default=0, null=True) platform = models.CharField(max_length=64, null=True) @@ -586,6 +585,7 @@ class Group(EventBase): time_spent_count = BoundedIntegerField(default=0) score = BoundedIntegerField(default=0) is_public = models.NullBooleanField(default=False, null=True) + data = GzippedDictField(blank=True, null=True) objects = GroupManager() @@ -729,6 +729,7 @@ class Event(EventBase): time_spent = models.FloatField(null=True) server_name = models.CharField(max_length=128, db_index=True, null=True) site = models.CharField(max_length=128, db_index=True, null=True) + data = NodeField(blank=True, null=True) objects = BaseManager() @@ -743,7 +744,7 @@ class Meta: @memoize def interfaces(self): result = [] - for key, data in self.node_data.iteritems(): + for key, data in self.data.iteritems(): if '.' not in key: continue @@ -760,10 +761,6 @@ def interfaces(self): return SortedDict((k, v) for k, v in sorted(result, key=lambda x: x[1].get_score(), reverse=True)) - @property - def node_data(self): - assert hasattr(self, '_node_data_cache'), 'missing node data cache' - def bind_node_data(self): from sentry import app From 7904d859bafd67074dde257bfc15fc2d7f4b5a59 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 14:17:15 +0900 Subject: [PATCH 08/34] Add set/create tests and ensure create returns node id --- src/sentry/nodestore/base.py | 3 ++- tests/sentry/nodestore/django/backend/tests.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 094aafdaeb6abd..1cc47f6ecd1da9 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -14,7 +14,8 @@ class NodeStorage(object): def create(self, data, timestamp=None): node_id = uuid.uuid4().hex - return self.set(node_id, data, timestamp) + self.set(node_id, data, timestamp) + return node_id def get(self, id): raise NotImplementedError diff --git a/tests/sentry/nodestore/django/backend/tests.py b/tests/sentry/nodestore/django/backend/tests.py index 6c16e35f6a33e9..58689095c6ee89 100644 --- a/tests/sentry/nodestore/django/backend/tests.py +++ b/tests/sentry/nodestore/django/backend/tests.py @@ -42,3 +42,19 @@ def test_get_multi(self): 'd2502ebbd7df41ceba8d3275595cac33', '5394aa025b8e401ca6bc3ddee3130edc' ]) assert result == dict((n.id, n.data) for n in nodes) + + def test_set(self): + self.ns.set('d2502ebbd7df41ceba8d3275595cac33', { + 'foo': 'bar', + }) + assert Node.objects.get(id='d2502ebbd7df41ceba8d3275595cac33').data == { + 'foo': 'bar', + } + + def test_create(self): + node_id = self.ns.create({ + 'foo': 'bar', + }) + assert Node.objects.get(id=node_id).data == { + 'foo': 'bar', + } From 9f0480de7f158194dfb4c647962bed4e56a3e2b0 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 15:38:49 +0900 Subject: [PATCH 09/34] Automatically manage node data --- src/sentry/db/models/fields/node.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/sentry/db/models/fields/node.py b/src/sentry/db/models/fields/node.py index 62f8d56d9fadb4..9079b9e4332985 100644 --- a/src/sentry/db/models/fields/node.py +++ b/src/sentry/db/models/fields/node.py @@ -49,7 +49,7 @@ def __repr__(self): if self._node_data: return '<%s: id=%s data=%r>' % ( cls_name, self.id, repr(self._node_data)) - return '<%s: id=%s>' % (self.id,) + return '<%s: id=%s>' % (cls_name, self.id,) @memoize def data(self): @@ -58,7 +58,7 @@ def data(self): return self._node_data def bind_node_data(self, data): - self.data = data + self._node_data = data class NodeField(GzippedDictField): @@ -88,13 +88,19 @@ def to_python(self, value): return NodeData(node_id, data) def get_prep_value(self, value): + from sentry import app + if not value and self.null: # save ourselves some storage return None - if value.id: - result = { - 'node_id': value.id - } + + # TODO(dcramer): we should probably do this more intelligently + # and manually + if not value.id: + value.id = app.nodestore.create(value.data) else: - result = value.data - return compress(pickle.dumps(result)) + app.nodestore.set(value.id, value.data) + + return compress(pickle.dumps({ + 'node_id': value.id + })) From 733452d58df4435b3928b72f6e5cff373120cc30 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 15:43:39 +0900 Subject: [PATCH 10/34] Dont hard error outside of test env for missing data --- src/sentry/db/models/fields/node.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/sentry/db/models/fields/node.py b/src/sentry/db/models/fields/node.py index 9079b9e4332985..25bdb3c0417c1a 100644 --- a/src/sentry/db/models/fields/node.py +++ b/src/sentry/db/models/fields/node.py @@ -10,6 +10,7 @@ import collections import logging +import warnings from django.db import models @@ -53,9 +54,16 @@ def __repr__(self): @memoize def data(self): - if self._node_data is None: - raise Exception('Must populate node data first') - return self._node_data + from sentry import app + + if self._node_data is not None: + return self._node_data + + elif self.id: + warnings.warn('You should populate node data before accessing it.') + return app.nodestore.get(self.id) or {} + + return {} def bind_node_data(self, data): self._node_data = data From e929d81db32d8fc66985c4b1639c53617d413ff7 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sun, 15 Sep 2013 16:05:46 +0900 Subject: [PATCH 11/34] Bind node data where used --- setup.py | 1 - src/sentry/db/models/fields/node.py | 2 +- src/sentry/db/models/manager.py | 16 ++++++++++++++++ src/sentry/nodestore/base.py | 5 ++++- src/sentry/nodestore/django/models.py | 4 +--- src/sentry/nodestore/migrations/0001_initial.py | 6 +++--- src/sentry/web/frontend/events.py | 2 ++ src/sentry/web/frontend/groups.py | 6 ++++++ 8 files changed, 33 insertions(+), 9 deletions(-) diff --git a/setup.py b/setup.py index 19b844aea56d69..47b77f84f054aa 100755 --- a/setup.py +++ b/setup.py @@ -72,7 +72,6 @@ 'django-picklefield>=0.3.0,<0.4.0', 'django-static-compiler>=0.3.0,<0.4.0', 'django-templatetag-sugar>=0.1.0,<0.2.0', - 'django-uuidfield==0.4.0', 'gunicorn>=0.17.2,<0.18.0', 'logan>=0.5.7,<0.6.0', 'nydus>=0.10.0,<0.11.0', diff --git a/src/sentry/db/models/fields/node.py b/src/sentry/db/models/fields/node.py index 25bdb3c0417c1a..ac90193a733455 100644 --- a/src/sentry/db/models/fields/node.py +++ b/src/sentry/db/models/fields/node.py @@ -65,7 +65,7 @@ def data(self): return {} - def bind_node_data(self, data): + def bind_data(self, data): self._node_data = data diff --git a/src/sentry/db/models/manager.py b/src/sentry/db/models/manager.py index 2fbbc6325b5e7e..3fd69100aaa08f 100644 --- a/src/sentry/db/models/manager.py +++ b/src/sentry/db/models/manager.py @@ -224,3 +224,19 @@ def get_from_cache(self, **kwargs): def create_or_update(self, **kwargs): return create_or_update(self.model, **kwargs) + + def bind_nodes(self, object_list, *node_names): + from sentry import app + + object_node_list = [] + for name in node_names: + object_node_list.extend((getattr(i, name) for i in object_list if getattr(i, name).id)) + + node_ids = [n.id for n in object_node_list] + if not node_ids: + return + + node_results = app.nodestore.get_multi(node_ids) + + for node in object_node_list: + node.bind_data(node_results.get(node.id) or {}) diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 1cc47f6ecd1da9..7a2adeaf8aac2d 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -13,7 +13,7 @@ class NodeStorage(object): def create(self, data, timestamp=None): - node_id = uuid.uuid4().hex + node_id = self.generate_id() self.set(node_id, data, timestamp) return node_id @@ -32,3 +32,6 @@ def set(self, id, data, timestamp=None): def set_multi(self, values): for v in values: self.set(**v) + + def generate_id(self): + return uuid.uuid4().hex diff --git a/src/sentry/nodestore/django/models.py b/src/sentry/nodestore/django/models.py index 3ce6dd1bcfbc8a..d4d5f1d82a6f27 100644 --- a/src/sentry/nodestore/django/models.py +++ b/src/sentry/nodestore/django/models.py @@ -8,8 +8,6 @@ from __future__ import absolute_import -from uuidfield import UUIDField - from django.db import models from django.utils import timezone @@ -18,7 +16,7 @@ class Node(BaseModel): - id = UUIDField(auto=True, primary_key=True) + id = models.CharField(max_length=40, primary_key=True) data = GzippedDictField() timestamp = models.DateTimeField(default=timezone.now) diff --git a/src/sentry/nodestore/migrations/0001_initial.py b/src/sentry/nodestore/migrations/0001_initial.py index 5c0254fefee90e..1091e273ea5e09 100644 --- a/src/sentry/nodestore/migrations/0001_initial.py +++ b/src/sentry/nodestore/migrations/0001_initial.py @@ -10,7 +10,7 @@ class Migration(SchemaMigration): def forwards(self, orm): # Adding model 'Node' db.create_table(u'nodestore_node', ( - ('id', self.gf('uuidfield.fields.UUIDField')(unique=True, max_length=32, primary_key=True)), + ('id', self.gf('django.db.models.fields.CharField')(unique=True, max_length=40, primary_key=True)), ('data', self.gf('django.db.models.fields.TextField')()), ('timestamp', self.gf('django.db.models.fields.DateTimeField')(default=datetime.datetime.now)), )) @@ -26,9 +26,9 @@ def backwards(self, orm): 'nodestore.node': { 'Meta': {'object_name': 'Node'}, 'data': ('django.db.models.fields.TextField', [], {}), - 'id': ('uuidfield.fields.UUIDField', [], {'unique': 'True', 'max_length': '32', 'primary_key': 'True'}), + 'id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '40', 'primary_key': 'True'}), 'timestamp': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}) } } - complete_apps = ['nodestore'] \ No newline at end of file + complete_apps = ['nodestore'] diff --git a/src/sentry/web/frontend/events.py b/src/sentry/web/frontend/events.py index 851e9971f4d149..748c9508be6c37 100644 --- a/src/sentry/web/frontend/events.py +++ b/src/sentry/web/frontend/events.py @@ -25,6 +25,8 @@ def replay_event(request, team, project, group, event_id): except Event.DoesNotExist: return HttpResponseRedirect(reverse('sentry')) + Event.objects.bind_nodes([event], 'data') + interfaces = event.interfaces if 'sentry.interfaces.Http' not in interfaces: # TODO: show a proper error diff --git a/src/sentry/web/frontend/groups.py b/src/sentry/web/frontend/groups.py index 3ea8a2afc04cdb..0d7178173fbd00 100644 --- a/src/sentry/web/frontend/groups.py +++ b/src/sentry/web/frontend/groups.py @@ -388,6 +388,8 @@ def group(request, team, project, group, event_id=None): else: event = group.get_latest_event() or Event() + Event.objects.bind_nodes([event], 'data') + # bind params to group in case they get hit event.group = group event.project = project @@ -503,6 +505,8 @@ def group_event_list_json(request, team, project, group_id): events = group.event_set.order_by('-id')[:limit] + Event.objects.bind_nodes(events, 'data') + return HttpResponse(json.dumps([event.as_dict() for event in events]), mimetype='application/json') @@ -517,6 +521,8 @@ def group_event_details_json(request, team, project, group_id, event_id_or_lates else: event = get_object_or_404(group.event_set, pk=event_id_or_latest) + Event.objects.bind_nodes([event], 'data') + return HttpResponse(json.dumps(event.as_dict()), mimetype='application/json') From ba88e24373eef35b6f115964cd8d3d9b07782153 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 17 Oct 2013 21:52:26 -0700 Subject: [PATCH 12/34] Add multi backend to nodestore --- src/sentry/nodestore/multi/__init__.py | 7 +++ src/sentry/nodestore/multi/backend.py | 73 ++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 src/sentry/nodestore/multi/__init__.py create mode 100644 src/sentry/nodestore/multi/backend.py diff --git a/src/sentry/nodestore/multi/__init__.py b/src/sentry/nodestore/multi/__init__.py new file mode 100644 index 00000000000000..23e12bb3775a05 --- /dev/null +++ b/src/sentry/nodestore/multi/__init__.py @@ -0,0 +1,7 @@ +""" +sentry.nodestore.multi +~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" diff --git a/src/sentry/nodestore/multi/backend.py b/src/sentry/nodestore/multi/backend.py new file mode 100644 index 00000000000000..d5c3ee46ca6c0a --- /dev/null +++ b/src/sentry/nodestore/multi/backend.py @@ -0,0 +1,73 @@ +""" +sentry.nodestore.multi.backend +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + +import random + +from django.utils import timezone + +from sentry.nodestore.base import NodeStorage +from sentry.utils.imports import import_string + + +class MultiNodeStorage(NodeStorage): + """ + A backend which will write to multiple backends, and read from a random + choice. + + This is not intended for consistency, but is instead designed to allow you + to dual-write for purposes of migrations. + + >>> MultiNodeStorage(backends=[ + >>> ('sentry.nodestore.django.backend.DjangoNodeStorage', {}), + >>> ('sentry.nodestore.riak.backend.RiakNodeStorage', {}), + >>> ]) + """ + def __init__(self, backends, **kwargs): + assert backends, "you should provide at least one backend" + + self.backends = [] + for backend, backend_options in backends: + cls = import_string(backend) + self.backends.append(cls(**backend_options)) + super(MultiNodeStorage, self).__init__(**kwargs) + + def get(self, id): + # just fetch it from a random backend, we're not aiming for consistency + backend = random.choice(self.backends) + return backend.get(id=id) + + def get_multi(self, id_list): + backend = random.choice(self.backends) + return backend.get_multi(id_list=id_list) + + def set(self, id, data, timestamp=None): + if timestamp is None: + timestamp = timezone.now() + + should_raise = False + for backend in self.backends: + try: + backend.set(id=id, data=data, timestamp=timestamp) + except Exception: + should_raise = True + + if should_raise: + raise + + def set_multi(self, values): + should_raise = False + for backend in self.backends: + try: + backend.set_multi(values) + except Exception: + should_raise = True + + if should_raise: + raise From 2e7d1f5d81bb71760deb101fb7961f68e29827e0 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 17 Oct 2013 22:56:16 -0700 Subject: [PATCH 13/34] Remove timestamp from settable node commands, add set_multi tests, and minor cleanup --- src/sentry/nodestore/base.py | 31 ++++++++++++++++--- src/sentry/nodestore/django/backend.py | 12 +++---- src/sentry/nodestore/multi/backend.py | 9 ++---- .../sentry/nodestore/django/backend/tests.py | 16 ++++++++++ 4 files changed, 49 insertions(+), 19 deletions(-) diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 7a2adeaf8aac2d..06cd44845f3655 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -12,26 +12,47 @@ class NodeStorage(object): - def create(self, data, timestamp=None): + def create(self, data): + """ + >>> key = nodestore.create({'foo': 'bar'}) + """ node_id = self.generate_id() - self.set(node_id, data, timestamp) + self.set(node_id, data) return node_id def get(self, id): + """ + >>> data = nodestore.get('key1') + >>> print data + """ raise NotImplementedError def get_multi(self, id_list): + """ + >>> data_map = nodestore.get_multi(['key1', 'key2') + >>> print 'key1', data_map['key1'] + >>> print 'key2', data_map['key2'] + """ return dict( (id, self.get(id)) for id in id_list ) - def set(self, id, data, timestamp=None): + def set(self, id, data): + """ + >>> nodestore.set('key1', {'foo': 'bar'}) + """ raise NotImplementedError def set_multi(self, values): - for v in values: - self.set(**v) + """ + >>> nodestore.set_multi({ + >>> 'key1': {'foo': 'bar'}, + >>> 'key2': {'foo': 'baz'}, + >>> }) + """ + for id, data in values.iteritems(): + self.set(id=id, data=data) def generate_id(self): return uuid.uuid4().hex diff --git a/src/sentry/nodestore/django/backend.py b/src/sentry/nodestore/django/backend.py index 259e42ba730e2a..cf5f1381e24f42 100644 --- a/src/sentry/nodestore/django/backend.py +++ b/src/sentry/nodestore/django/backend.py @@ -29,14 +29,12 @@ def get_multi(self, id_list): for n in Node.objects.filter(id__in=id_list) ) - def set(self, id, data, timestamp=None): + def set(self, id, data): create_or_update( Node, id=id, - data=data, - timestamp=timestamp or timezone.now() + defaults={ + 'data': data, + 'timestamp': timezone.now(), + }, ) - - def set_multi(self, values): - for v in values: - self.set(**v) diff --git a/src/sentry/nodestore/multi/backend.py b/src/sentry/nodestore/multi/backend.py index d5c3ee46ca6c0a..9bdcaa050ac5d3 100644 --- a/src/sentry/nodestore/multi/backend.py +++ b/src/sentry/nodestore/multi/backend.py @@ -10,8 +10,6 @@ import random -from django.utils import timezone - from sentry.nodestore.base import NodeStorage from sentry.utils.imports import import_string @@ -47,14 +45,11 @@ def get_multi(self, id_list): backend = random.choice(self.backends) return backend.get_multi(id_list=id_list) - def set(self, id, data, timestamp=None): - if timestamp is None: - timestamp = timezone.now() - + def set(self, id, data): should_raise = False for backend in self.backends: try: - backend.set(id=id, data=data, timestamp=timestamp) + backend.set(id=id, data=data) except Exception: should_raise = True diff --git a/tests/sentry/nodestore/django/backend/tests.py b/tests/sentry/nodestore/django/backend/tests.py index 58689095c6ee89..1b13f3ff71a57d 100644 --- a/tests/sentry/nodestore/django/backend/tests.py +++ b/tests/sentry/nodestore/django/backend/tests.py @@ -51,6 +51,22 @@ def test_set(self): 'foo': 'bar', } + def test_set_multi(self): + self.ns.set_multi({ + 'd2502ebbd7df41ceba8d3275595cac33': { + 'foo': 'bar', + }, + '5394aa025b8e401ca6bc3ddee3130edc': { + 'foo': 'baz', + }, + }) + assert Node.objects.get(id='d2502ebbd7df41ceba8d3275595cac33').data == { + 'foo': 'bar', + } + assert Node.objects.get(id='5394aa025b8e401ca6bc3ddee3130edc').data == { + 'foo': 'baz', + } + def test_create(self): node_id = self.ns.create({ 'foo': 'bar', From 5360c7cf8821e69afc6c4425b95d32f33bf33338 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 17 Oct 2013 22:56:26 -0700 Subject: [PATCH 14/34] First pass at Riak nodestore backend --- src/sentry/nodestore/riak/__init__.py | 7 ++++ src/sentry/nodestore/riak/backend.py | 50 +++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 src/sentry/nodestore/riak/__init__.py create mode 100644 src/sentry/nodestore/riak/backend.py diff --git a/src/sentry/nodestore/riak/__init__.py b/src/sentry/nodestore/riak/__init__.py new file mode 100644 index 00000000000000..0f526e2f155c52 --- /dev/null +++ b/src/sentry/nodestore/riak/__init__.py @@ -0,0 +1,7 @@ +""" +sentry.nodestore.riak +~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" diff --git a/src/sentry/nodestore/riak/backend.py b/src/sentry/nodestore/riak/backend.py new file mode 100644 index 00000000000000..4fe8319b1b8f7f --- /dev/null +++ b/src/sentry/nodestore/riak/backend.py @@ -0,0 +1,50 @@ +""" +sentry.nodestore.riak.backend +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:copyright: (c) 2010-2013 by the Sentry Team, see AUTHORS for more details. +:license: BSD, see LICENSE for more details. +""" + +from __future__ import absolute_import + +import riak +import riak.resolver + +from sentry.nodestore.base import NodeStorage + + +class RiakNodeStorage(NodeStorage): + """ + A Riak-based backend for storing node data. + + >>> RiakNodeStorage(nodes=[{'host':'127.0.0.1','http_port':8098}]) + """ + def __init__(self, nodes, bucket='nodes', **kwargs): + self.conn = riak.RiakClient(nodes=nodes, **kwargs) + self.bucket = self.conn.bucket( + bucket, resolver=riak.resolver.last_written_resolver) + super(RiakNodeStorage, self).__init__(**kwargs) + + def create(self, data): + obj = self.bucket.new(data=data) + obj.store() + return obj.key + + def get(self, id): + # just fetch it from a random backend, we're not aiming for consistency + obj = self.bucket.get(key=id) + if not obj: + return None + return obj.data + + def get_multi(self, id_list): + result = self.bucket.multiget(id_list) + return dict( + (obj.key, obj.data) + for obj in result + ) + + def set(self, id, data): + obj = self.bucket.new(key=id, data=data) + obj.store() From fce493869d449cc668ae12d5ad7a0d836b51b64e Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 14:27:23 -0700 Subject: [PATCH 15/34] Add Riak integration tests --- .travis.yml | 6 ++ tests/sentry/nodestore/riak/__init__.py | 0 .../sentry/nodestore/riak/backend/__init__.py | 0 tests/sentry/nodestore/riak/backend/tests.py | 55 +++++++++++++++++++ 4 files changed, 61 insertions(+) create mode 100644 tests/sentry/nodestore/riak/__init__.py create mode 100644 tests/sentry/nodestore/riak/backend/__init__.py create mode 100644 tests/sentry/nodestore/riak/backend/tests.py diff --git a/.travis.yml b/.travis.yml index 153d54c1d116c8..334da25976755a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,10 @@ language: python +services: + - memcached + - riak + - mysql + - postgresql + - redis-server python: - "2.6" - "2.7" diff --git a/tests/sentry/nodestore/riak/__init__.py b/tests/sentry/nodestore/riak/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/riak/backend/__init__.py b/tests/sentry/nodestore/riak/backend/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/riak/backend/tests.py b/tests/sentry/nodestore/riak/backend/tests.py new file mode 100644 index 00000000000000..4afce19a6e885b --- /dev/null +++ b/tests/sentry/nodestore/riak/backend/tests.py @@ -0,0 +1,55 @@ +# -*- coding: utf-8 -*- + +from __future__ import absolute_import + +import pytest + +from sentry.nodestore.riak.backend import RiakNodeStorage +from sentry.testutils import TestCase + + +def riak_is_available(): + import socket + try: + socket.create_connection(('127.0.0.1', 8098), 1.0) + except socket.error: + return False + else: + return True + + +@pytest.mark.skipif('not riak_is_available()', + reason="requires riak server running") +class RiakNodeStorageTest(TestCase): + def setUp(self): + self.ns = RiakNodeStorage(nodes=({ + 'host': '127.0.0.1', + 'http_port': 8098, + })) + + def test_integration(self): + node_id = self.ns.create({ + 'foo': 'bar', + }) + assert node_id is not None + + self.ns.set(node_id, { + 'foo': 'baz', + }) + + result = self.ns.get(node_id) + assert result == { + 'foo': 'baz', + } + + node_id2 = self.ns.create({ + 'foo': 'bar', + }) + + result = self.ns.get_multi([node_id, node_id2]) + assert result[node_id] == { + 'foo': 'baz', + } + assert result[node_id2] == { + 'foo': 'bar', + } From bd4417eea605ae1ac434d55779aa620bc242816a Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 14:30:55 -0700 Subject: [PATCH 16/34] Make require_riak a thing --- tests/sentry/nodestore/riak/backend/tests.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/sentry/nodestore/riak/backend/tests.py b/tests/sentry/nodestore/riak/backend/tests.py index 4afce19a6e885b..92aebb5881fc60 100644 --- a/tests/sentry/nodestore/riak/backend/tests.py +++ b/tests/sentry/nodestore/riak/backend/tests.py @@ -17,9 +17,12 @@ def riak_is_available(): else: return True +require_riak = pytest.mark.skipif( + 'not riak_is_available()', + reason="requires riak server running") -@pytest.mark.skipif('not riak_is_available()', - reason="requires riak server running") + +@require_riak class RiakNodeStorageTest(TestCase): def setUp(self): self.ns = RiakNodeStorage(nodes=({ From 8a9acbbb126ef59f75ebc20050e966d36155d091 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 14:31:50 -0700 Subject: [PATCH 17/34] Add riak to test deps --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index f82f8e36169977..d2d48aa55e4c94 100755 --- a/setup.py +++ b/setup.py @@ -57,6 +57,7 @@ 'nydus', 'mock>=0.8.0', 'redis', + 'riak', 'unittest2', ] From 4384f401f805afd2c8adf06960e8abc8ab629bd0 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 14:40:30 -0700 Subject: [PATCH 18/34] Fix list of nodes --- tests/sentry/nodestore/riak/backend/tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/sentry/nodestore/riak/backend/tests.py b/tests/sentry/nodestore/riak/backend/tests.py index 92aebb5881fc60..02896e9f868a82 100644 --- a/tests/sentry/nodestore/riak/backend/tests.py +++ b/tests/sentry/nodestore/riak/backend/tests.py @@ -25,10 +25,10 @@ def riak_is_available(): @require_riak class RiakNodeStorageTest(TestCase): def setUp(self): - self.ns = RiakNodeStorage(nodes=({ + self.ns = RiakNodeStorage(nodes=[{ 'host': '127.0.0.1', 'http_port': 8098, - })) + }]) def test_integration(self): node_id = self.ns.create({ From 0517ecc4e4955483bd8f845dce6c039a1eec1d8b Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 14:44:06 -0700 Subject: [PATCH 19/34] Specify resolver at client level --- src/sentry/nodestore/riak/backend.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sentry/nodestore/riak/backend.py b/src/sentry/nodestore/riak/backend.py index 4fe8319b1b8f7f..608ed5fdda19e0 100644 --- a/src/sentry/nodestore/riak/backend.py +++ b/src/sentry/nodestore/riak/backend.py @@ -20,10 +20,11 @@ class RiakNodeStorage(NodeStorage): >>> RiakNodeStorage(nodes=[{'host':'127.0.0.1','http_port':8098}]) """ - def __init__(self, nodes, bucket='nodes', **kwargs): - self.conn = riak.RiakClient(nodes=nodes, **kwargs) - self.bucket = self.conn.bucket( - bucket, resolver=riak.resolver.last_written_resolver) + def __init__(self, nodes, bucket='nodes', + resolver=riak.resolver.last_written_resolver, **kwargs): + self.conn = riak.RiakClient( + nodes=nodes, resolver=resolver, **kwargs) + self.bucket = self.conn.bucket(bucket) super(RiakNodeStorage, self).__init__(**kwargs) def create(self, data): From d68dd0510da7f70d6f40eed5b55dcfcd252bb796 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 15:35:02 -0700 Subject: [PATCH 20/34] Move pytest riak helper into testutils --- src/sentry/testutils.py | 17 +++++++++++++++++ tests/sentry/nodestore/riak/backend/tests.py | 20 ++------------------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/sentry/testutils.py b/src/sentry/testutils.py index 12086666089721..607f0f99e638e9 100644 --- a/src/sentry/testutils.py +++ b/src/sentry/testutils.py @@ -9,6 +9,8 @@ from __future__ import absolute_import import base64 +import pytest + from exam import Exam, fixture, before # NOQA from functools import wraps @@ -272,3 +274,18 @@ def wrapped(*args, **kwargs): finally: app.conf.CELERY_ALWAYS_EAGER = prev return wrapped + + +def riak_is_available(): + import socket + try: + socket.create_connection(('127.0.0.1', 8098), 1.0) + except socket.error: + return False + else: + return True + + +requires_riak = pytest.mark.skipif( + 'not riak_is_available()', + reason="requires riak server running") diff --git a/tests/sentry/nodestore/riak/backend/tests.py b/tests/sentry/nodestore/riak/backend/tests.py index 02896e9f868a82..92a76302181ec2 100644 --- a/tests/sentry/nodestore/riak/backend/tests.py +++ b/tests/sentry/nodestore/riak/backend/tests.py @@ -2,27 +2,11 @@ from __future__ import absolute_import -import pytest - from sentry.nodestore.riak.backend import RiakNodeStorage -from sentry.testutils import TestCase - - -def riak_is_available(): - import socket - try: - socket.create_connection(('127.0.0.1', 8098), 1.0) - except socket.error: - return False - else: - return True - -require_riak = pytest.mark.skipif( - 'not riak_is_available()', - reason="requires riak server running") +from sentry.testutils import TestCase, requires_riak -@require_riak +@requires_riak class RiakNodeStorageTest(TestCase): def setUp(self): self.ns = RiakNodeStorage(nodes=[{ From d2c8d134677922a75a174194350a6bd9f42c16fd Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 15:46:30 -0700 Subject: [PATCH 21/34] Fix call to riak check --- src/sentry/testutils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/testutils.py b/src/sentry/testutils.py index 607f0f99e638e9..163a851faa07f9 100644 --- a/src/sentry/testutils.py +++ b/src/sentry/testutils.py @@ -287,5 +287,5 @@ def riak_is_available(): requires_riak = pytest.mark.skipif( - 'not riak_is_available()', + lambda x: not riak_is_available(), reason="requires riak server running") From 1b25a920767451d9378bbe5776967075b2094868 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 15:46:35 -0700 Subject: [PATCH 22/34] Tests for multi backend --- src/sentry/nodestore/multi/backend.py | 9 +- tests/sentry/nodestore/multi/__init__.py | 0 .../nodestore/multi/backend/__init__.py | 0 tests/sentry/nodestore/multi/backend/tests.py | 82 +++++++++++++++++++ 4 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 tests/sentry/nodestore/multi/__init__.py create mode 100644 tests/sentry/nodestore/multi/backend/__init__.py create mode 100644 tests/sentry/nodestore/multi/backend/tests.py diff --git a/src/sentry/nodestore/multi/backend.py b/src/sentry/nodestore/multi/backend.py index 9bdcaa050ac5d3..99ad574ba5364e 100644 --- a/src/sentry/nodestore/multi/backend.py +++ b/src/sentry/nodestore/multi/backend.py @@ -32,14 +32,15 @@ def __init__(self, backends, **kwargs): self.backends = [] for backend, backend_options in backends: - cls = import_string(backend) - self.backends.append(cls(**backend_options)) + if isinstance(backend, basestring): + backend = import_string(backend) + self.backends.append(backend(**backend_options)) super(MultiNodeStorage, self).__init__(**kwargs) def get(self, id): # just fetch it from a random backend, we're not aiming for consistency backend = random.choice(self.backends) - return backend.get(id=id) + return backend.get(id) def get_multi(self, id_list): backend = random.choice(self.backends) @@ -49,7 +50,7 @@ def set(self, id, data): should_raise = False for backend in self.backends: try: - backend.set(id=id, data=data) + backend.set(id, data) except Exception: should_raise = True diff --git a/tests/sentry/nodestore/multi/__init__.py b/tests/sentry/nodestore/multi/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/multi/backend/__init__.py b/tests/sentry/nodestore/multi/backend/__init__.py new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tests/sentry/nodestore/multi/backend/tests.py b/tests/sentry/nodestore/multi/backend/tests.py new file mode 100644 index 00000000000000..1a282ff22aa36e --- /dev/null +++ b/tests/sentry/nodestore/multi/backend/tests.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- + +from __future__ import absolute_import + +from sentry.nodestore.base import NodeStorage +from sentry.nodestore.multi.backend import MultiNodeStorage +from sentry.testutils import TestCase + + +class InMemoryBackend(NodeStorage): + def __init__(self): + self._data = {} + + def set(self, id, data): + self._data[id] = data + + def get(self, id): + return self._data.get(id) + + +class MultiNodeStorageTest(TestCase): + def setUp(self): + self.ns = MultiNodeStorage([ + (InMemoryBackend, {}), + (InMemoryBackend, {}), + ]) + + def test_basic_integration(self): + node_id = self.ns.create({ + 'foo': 'bar', + }) + assert node_id is not None + for backend in self.ns.backends: + assert backend.get(node_id) == { + 'foo': 'bar', + } + + self.ns.set(node_id, { + 'foo': 'baz', + }) + for backend in self.ns.backends: + assert backend.get(node_id) == { + 'foo': 'baz', + } + + result = self.ns.get(node_id) + assert result == { + 'foo': 'baz', + } + + node_id2 = self.ns.create({ + 'foo': 'bar', + }) + for backend in self.ns.backends: + assert backend.get(node_id2) == { + 'foo': 'bar', + } + + result = self.ns.get_multi([node_id, node_id2]) + assert result[node_id] == { + 'foo': 'baz', + } + assert result[node_id2] == { + 'foo': 'bar', + } + + result = self.ns.set_multi({ + node_id: { + 'foo': 'biz', + }, + node_id2: { + 'foo': 'bir', + }, + }) + + for backend in self.ns.backends: + assert backend.get(node_id) == { + 'foo': 'biz', + } + assert backend.get(node_id2) == { + 'foo': 'bir', + } From b0af57bdc79ce6db0a54daf5d5a00f393fe6e2e5 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:21:40 -0700 Subject: [PATCH 23/34] Add tests for node data transitions --- src/sentry/models.py | 10 ---------- tests/sentry/models/tests.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/sentry/models.py b/src/sentry/models.py index da974933fb91b3..32500e617ca21d 100644 --- a/src/sentry/models.py +++ b/src/sentry/models.py @@ -756,16 +756,6 @@ def interfaces(self): return SortedDict((k, v) for k, v in sorted(result, key=lambda x: x[1].get_score(), reverse=True)) - def bind_node_data(self): - from sentry import app - - node_id = self._data.get('data_nid') - - if not node_id: - return self._data - - self._node_data_cache = app.nodestore.get(node_id) - def get_version(self): if not self.data: return diff --git a/tests/sentry/models/tests.py b/tests/sentry/models/tests.py index 36062e2c4445d8..b00174517ea945 100644 --- a/tests/sentry/models/tests.py +++ b/tests/sentry/models/tests.py @@ -8,13 +8,17 @@ from django.conf import settings from django.core import mail from django.core.urlresolvers import reverse +from django.db import connection from django.utils import timezone from sentry.constants import MINUTE_NORMALIZATION +from sentry.db.models.fields.node import NodeData from sentry.models import ( Project, ProjectKey, Group, Event, Team, GroupTag, GroupCountByMinute, TagValue, PendingTeamMember, LostPasswordHash, Alert, User, create_default_project) from sentry.testutils import TestCase, fixture +from sentry.utils.compat import pickle +from sentry.utils.strings import compress class ProjectTest(TestCase): @@ -169,3 +173,33 @@ def test_simple(self): team = project.team assert team.owner == user assert team.slug == 'sentry' + + +class EventNodeStoreTest(TestCase): + def test_does_transition_data_to_node(self): + group = self.group + data = {'key': 'value'} + + cursor = connection.cursor() + cursor.execute( + "INSERT INTO sentry_message (group_id, project_id, data, logger, level, message, checksum, datetime) VALUES(%s, %s, %s, '', 0, %s, %s, %s)", + [group.id, group.project_id, compress(pickle.dumps(data)), 'test', 'a' * 32, timezone.now()] + ) + event_id = cursor.lastrowid + event = Event.objects.get(id=event_id) + assert type(event.data) == NodeData + assert event.data == data + assert event.data.id is None + + event.save() + + assert event.data == data + assert event.data.id is not None + + node_id = event.data.id + event = Event.objects.get(id=event_id) + + Event.objects.bind_nodes([event], 'data') + + assert event.data == data + assert event.data.id == node_id From eccd884717213fb69cd205ddd2227b75350e5b5f Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:26:50 -0700 Subject: [PATCH 24/34] Expand test coverage for event list and json feed --- src/sentry/testutils.py | 16 +++++++++++----- tests/sentry/web/frontend/groups/tests.py | 18 ++++++++++++++---- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/sentry/testutils.py b/src/sentry/testutils.py index 163a851faa07f9..6dbdf60bbe2aa4 100644 --- a/src/sentry/testutils.py +++ b/src/sentry/testutils.py @@ -129,12 +129,18 @@ def group(self): @fixture def event(self): + return self.create_event(event_id='a' * 32) + + def create_event(self, event_id, **kwargs): + if 'group' not in kwargs: + kwargs['group'] = self.group + kwargs.setdefault('project', kwargs['group'].project) + kwargs.setdefault('message', 'Foo bar') + kwargs.setdefault('data', LEGACY_DATA) + return Event.objects.create( - event_id='a' * 32, - group=self.group, - message='Foo bar', - project=self.project, - data=LEGACY_DATA, + event_id=event_id, + **kwargs ) def assertRequiresAuthentication(self, path, method='GET'): diff --git a/tests/sentry/web/frontend/groups/tests.py b/tests/sentry/web/frontend/groups/tests.py index c7054223de42b1..9fec163bfbb390 100644 --- a/tests/sentry/web/frontend/groups/tests.py +++ b/tests/sentry/web/frontend/groups/tests.py @@ -113,6 +113,9 @@ def path(self): }) def test_does_render(self): + event = self.create_event(event_id='a' * 32) + event2 = self.create_event(event_id='b' * 32) + self.login() resp = self.client.get(self.path) assert resp.status_code == 200 @@ -124,6 +127,10 @@ def test_does_render(self): assert resp.context['project'] == self.project assert resp.context['team'] == self.team assert resp.context['group'] == self.group + event_list = resp.context['event_list'] + assert len(event_list) == 2 + assert event_list[0] == event2 + assert event_list[1] == event class GroupTagListTest(TestCase): @@ -185,14 +192,17 @@ def path(self): def test_does_render(self): self.login() - # HACK: force fixture creation - self.event + + event = self.create_event(event_id='a' * 32) + event2 = self.create_event(event_id='b' * 32) + resp = self.client.get(self.path) assert resp.status_code == 200 assert resp['Content-Type'] == 'application/json' data = json.loads(resp.content) - assert len(data) == 1 - assert data[0]['id'] == str(self.event.event_id) + assert len(data) == 2 + assert data[0]['id'] == str(event2.event_id) + assert data[1]['id'] == str(event.event_id) def test_does_not_allow_beyond_limit(self): self.login() From ac59037785163828cad970bae6c020ec5207b270 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:27:29 -0700 Subject: [PATCH 25/34] Ensure nodes are bound in event list --- src/sentry/web/frontend/groups.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sentry/web/frontend/groups.py b/src/sentry/web/frontend/groups.py index 9d61434e3fdcb0..0fa0cf7f3a4569 100644 --- a/src/sentry/web/frontend/groups.py +++ b/src/sentry/web/frontend/groups.py @@ -509,6 +509,8 @@ def group_tag_details(request, team, project, group, tag_name): def group_event_list(request, team, project, group): event_list = group.event_set.all().order_by('-datetime') + Event.objects.bind_nodes(event_list, 'data') + return render_with_group_context(group, 'sentry/groups/event_list.html', { 'event_list': event_list, 'page': 'event_list', From f85e9eb73a90f2e5cb63f334a2fe22dc5883500b Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:43:45 -0700 Subject: [PATCH 26/34] Ensure datetime reflects accurate ordering for tests --- tests/sentry/web/frontend/groups/tests.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/sentry/web/frontend/groups/tests.py b/tests/sentry/web/frontend/groups/tests.py index 9fec163bfbb390..3c9e65829d992d 100644 --- a/tests/sentry/web/frontend/groups/tests.py +++ b/tests/sentry/web/frontend/groups/tests.py @@ -113,8 +113,10 @@ def path(self): }) def test_does_render(self): - event = self.create_event(event_id='a' * 32) - event2 = self.create_event(event_id='b' * 32) + event = self.create_event( + event_id='a' * 32, datetime=timezone.now() - timedelta(minutes=1)) + event2 = self.create_event( + event_id='b' * 32, datetime=timezone.now()) self.login() resp = self.client.get(self.path) @@ -193,8 +195,10 @@ def path(self): def test_does_render(self): self.login() - event = self.create_event(event_id='a' * 32) - event2 = self.create_event(event_id='b' * 32) + event = self.create_event( + event_id='a' * 32, datetime=timezone.now() - timedelta(minutes=1)) + event2 = self.create_event( + event_id='b' * 32, datetime=timezone.now()) resp = self.client.get(self.path) assert resp.status_code == 200 From 5d15a3ee0de1abe135d7041ff558c2d6a781acfd Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:50:57 -0700 Subject: [PATCH 27/34] Support return id clauses in test case --- tests/sentry/models/tests.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/sentry/models/tests.py b/tests/sentry/models/tests.py index b00174517ea945..5fe80555bec952 100644 --- a/tests/sentry/models/tests.py +++ b/tests/sentry/models/tests.py @@ -180,11 +180,22 @@ def test_does_transition_data_to_node(self): group = self.group data = {'key': 'value'} + query_bits = [ + "INSERT INTO sentry_message (group_id, project_id, data, logger, level, message, checksum, datetime)", + "VALUES(%s, %s, %s, '', 0, %s, %s, %s)", + ] + params = [group.id, group.project_id, compress(pickle.dumps(data)), 'test', 'a' * 32, timezone.now()] + + # This is pulled from SQLInsertCompiler + if connection.features.can_return_id_from_insert: + r_fmt, r_params = connection.ops.return_insert_id() + if r_fmt: + query_bits.append(r_fmt % 'id') + params += r_params + cursor = connection.cursor() - cursor.execute( - "INSERT INTO sentry_message (group_id, project_id, data, logger, level, message, checksum, datetime) VALUES(%s, %s, %s, '', 0, %s, %s, %s)", - [group.id, group.project_id, compress(pickle.dumps(data)), 'test', 'a' * 32, timezone.now()] - ) + cursor.execute(' '.join(query_bits), params) + event_id = cursor.lastrowid event = Event.objects.get(id=event_id) assert type(event.data) == NodeData From 7f81b0010d38f30a6dc560169bc2acf0dfe84841 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:58:37 -0700 Subject: [PATCH 28/34] Index Node.timestamp --- src/sentry/nodestore/django/models.py | 2 +- src/sentry/nodestore/migrations/0001_initial.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sentry/nodestore/django/models.py b/src/sentry/nodestore/django/models.py index d4d5f1d82a6f27..1c70e65913b71d 100644 --- a/src/sentry/nodestore/django/models.py +++ b/src/sentry/nodestore/django/models.py @@ -18,7 +18,7 @@ class Node(BaseModel): id = models.CharField(max_length=40, primary_key=True) data = GzippedDictField() - timestamp = models.DateTimeField(default=timezone.now) + timestamp = models.DateTimeField(default=timezone.now, db_index=True) __repr__ = sane_repr('timestamp') diff --git a/src/sentry/nodestore/migrations/0001_initial.py b/src/sentry/nodestore/migrations/0001_initial.py index 1091e273ea5e09..22ba0d08aa9b4e 100644 --- a/src/sentry/nodestore/migrations/0001_initial.py +++ b/src/sentry/nodestore/migrations/0001_initial.py @@ -12,7 +12,7 @@ def forwards(self, orm): db.create_table(u'nodestore_node', ( ('id', self.gf('django.db.models.fields.CharField')(unique=True, max_length=40, primary_key=True)), ('data', self.gf('django.db.models.fields.TextField')()), - ('timestamp', self.gf('django.db.models.fields.DateTimeField')(default=datetime.datetime.now)), + ('timestamp', self.gf('django.db.models.fields.DateTimeField')(default=datetime.datetime.now, db_index=True)), )) db.send_create_signal('nodestore', ['Node']) @@ -27,7 +27,7 @@ def backwards(self, orm): 'Meta': {'object_name': 'Node'}, 'data': ('django.db.models.fields.TextField', [], {}), 'id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '40', 'primary_key': 'True'}), - 'timestamp': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}) + 'timestamp': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now', 'db_index': 'True'}) } } From efafaf7295154e5f908e090860c188def293fd80 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 18:59:03 -0700 Subject: [PATCH 29/34] Add Node to cleanup --- src/sentry/tasks/cleanup.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sentry/tasks/cleanup.py b/src/sentry/tasks/cleanup.py index 09a82cf8945acb..c94b530cf8b562 100644 --- a/src/sentry/tasks/cleanup.py +++ b/src/sentry/tasks/cleanup.py @@ -28,6 +28,7 @@ def cleanup(days=30, project=None, chunk_size=1000, **kwargs): Group, Event, GroupCountByMinute, EventMapping, GroupTag, TagValue, ProjectCountByMinute, Alert, SearchDocument, Activity, LostPasswordHash) + from sentry.nodestore.django.models import Node GENERIC_DELETES = ( (SearchDocument, 'date_changed'), @@ -41,6 +42,7 @@ def cleanup(days=30, project=None, chunk_size=1000, **kwargs): (EventMapping, 'date_added'), # Group should probably be last (Group, 'last_seen'), + (Node, 'timestamp'), ) log = cleanup.get_logger() From 99e957eea2deb53d273f04481859f42301b0ca2f Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 19:11:35 -0700 Subject: [PATCH 30/34] Correct node deletions (they're not bound to projects) --- src/sentry/tasks/cleanup.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/sentry/tasks/cleanup.py b/src/sentry/tasks/cleanup.py index c94b530cf8b562..2e9e0ee37669cd 100644 --- a/src/sentry/tasks/cleanup.py +++ b/src/sentry/tasks/cleanup.py @@ -42,13 +42,20 @@ def cleanup(days=30, project=None, chunk_size=1000, **kwargs): (EventMapping, 'date_added'), # Group should probably be last (Group, 'last_seen'), - (Node, 'timestamp'), ) log = cleanup.get_logger() ts = timezone.now() - datetime.timedelta(days=days) + log.info("Removing expired values for %r", LostPasswordHash) + LostPasswordHash.objects.filter( + date_added__lte=timezone.now() - datetime.timedelta(days=1) + ).delete() + + log.info("Removing old Node values") + Node.objects.filter(timestamp__lte=ts) + # Remove types which can easily be bound to project + date for model, date_col in GENERIC_DELETES: log.info("Removing %r for days=%s project=%r", model, days, project or '*') @@ -60,8 +67,3 @@ def cleanup(days=30, project=None, chunk_size=1000, **kwargs): for obj in list(qs[:chunk_size]): log.info("Removing %r", obj) obj.delete() - - log.info("Removing expired values for %r", LostPasswordHash) - LostPasswordHash.objects.filter( - date_added__lte=timezone.now() - datetime.timedelta(days=1) - ).delete() From 26b3fe8375d4029cedf700b3c73ffb3c176360ec Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 19:11:48 -0700 Subject: [PATCH 31/34] Add node deletion support (automatic via group deletion) --- src/sentry/db/models/fields/node.py | 17 +++++++++++++++++ src/sentry/nodestore/base.py | 6 ++++++ src/sentry/nodestore/django/backend.py | 3 +++ src/sentry/nodestore/multi/backend.py | 11 +++++++++++ src/sentry/nodestore/riak/backend.py | 4 ++++ tests/sentry/nodestore/django/backend/tests.py | 13 ++++++++++++- 6 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/sentry/db/models/fields/node.py b/src/sentry/db/models/fields/node.py index ac90193a733455..56fd80c9f4566a 100644 --- a/src/sentry/db/models/fields/node.py +++ b/src/sentry/db/models/fields/node.py @@ -13,6 +13,7 @@ import warnings from django.db import models +from django.db.models.signals import post_delete from sentry.utils.cache import memoize from sentry.utils.compat import pickle @@ -76,6 +77,22 @@ class NodeField(GzippedDictField): """ __metaclass__ = models.SubfieldBase + def contribute_to_class(self, cls, name): + super(NodeField, self).contribute_to_class(cls, name) + post_delete.connect( + self.on_delete, + sender=self.model, + weak=False) + + def on_delete(self, instance, **kwargs): + from sentry import app + + value = getattr(instance, self.name) + if not value.id: + return + + app.nodestore.delete(value.id) + def to_python(self, value): if isinstance(value, basestring) and value: try: diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 06cd44845f3655..13911cd1189e36 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -20,6 +20,12 @@ def create(self, data): self.set(node_id, data) return node_id + def delete(self, id): + """ + >>> nodestore.delete('key1') + """ + raise NotImplementedError + def get(self, id): """ >>> data = nodestore.get('key1') diff --git a/src/sentry/nodestore/django/backend.py b/src/sentry/nodestore/django/backend.py index cf5f1381e24f42..49310e2184300c 100644 --- a/src/sentry/nodestore/django/backend.py +++ b/src/sentry/nodestore/django/backend.py @@ -17,6 +17,9 @@ class DjangoNodeStorage(NodeStorage): + def delete(self, id): + Node.objects.filter(id=id).delete() + def get(self, id): try: return Node.objects.get(id=id).data diff --git a/src/sentry/nodestore/multi/backend.py b/src/sentry/nodestore/multi/backend.py index 99ad574ba5364e..6c225795b5771e 100644 --- a/src/sentry/nodestore/multi/backend.py +++ b/src/sentry/nodestore/multi/backend.py @@ -67,3 +67,14 @@ def set_multi(self, values): if should_raise: raise + + def delete(self, id): + should_raise = False + for backend in self.backends: + try: + backend.delete(id) + except Exception: + should_raise = True + + if should_raise: + raise diff --git a/src/sentry/nodestore/riak/backend.py b/src/sentry/nodestore/riak/backend.py index 608ed5fdda19e0..2b9ad2faa21829 100644 --- a/src/sentry/nodestore/riak/backend.py +++ b/src/sentry/nodestore/riak/backend.py @@ -32,6 +32,10 @@ def create(self, data): obj.store() return obj.key + def delete(self, id): + obj = self.bucket.new(key=id) + obj.delete() + def get(self, id): # just fetch it from a random backend, we're not aiming for consistency obj = self.bucket.get(key=id) diff --git a/tests/sentry/nodestore/django/backend/tests.py b/tests/sentry/nodestore/django/backend/tests.py index 1b13f3ff71a57d..d44893831ca724 100644 --- a/tests/sentry/nodestore/django/backend/tests.py +++ b/tests/sentry/nodestore/django/backend/tests.py @@ -19,7 +19,7 @@ def test_get(self): } ) - result = self.ns.get('d2502ebbd7df41ceba8d3275595cac33') + result = self.ns.get(node.id) assert result == node.data def test_get_multi(self): @@ -74,3 +74,14 @@ def test_create(self): assert Node.objects.get(id=node_id).data == { 'foo': 'bar', } + + def test_delete(self): + node = Node.objects.create( + id='d2502ebbd7df41ceba8d3275595cac33', + data={ + 'foo': 'bar', + } + ) + + self.ns.delete(node.id) + assert not Node.objects.filter(id=node.id).exists() From f041ebc388ab2832a52d58546a3ef21c8ff533ff Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 19:20:08 -0700 Subject: [PATCH 32/34] Move node cleanup to backend --- src/sentry/nodestore/base.py | 3 +++ src/sentry/nodestore/django/backend.py | 3 +++ src/sentry/nodestore/multi/backend.py | 11 +++++++++++ src/sentry/nodestore/riak/backend.py | 5 +++++ src/sentry/tasks/cleanup.py | 8 ++++++-- 5 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/sentry/nodestore/base.py b/src/sentry/nodestore/base.py index 13911cd1189e36..5105ecbefa4f76 100644 --- a/src/sentry/nodestore/base.py +++ b/src/sentry/nodestore/base.py @@ -62,3 +62,6 @@ def set_multi(self, values): def generate_id(self): return uuid.uuid4().hex + + def cleanup(self, cutoff_timestamp): + raise NotImplementedError diff --git a/src/sentry/nodestore/django/backend.py b/src/sentry/nodestore/django/backend.py index 49310e2184300c..d4c612cc64b7c6 100644 --- a/src/sentry/nodestore/django/backend.py +++ b/src/sentry/nodestore/django/backend.py @@ -41,3 +41,6 @@ def set(self, id, data): 'timestamp': timezone.now(), }, ) + + def cleanup(self, cutoff_timestamp): + Node.objects.filter(timestamp__lte=cutoff_timestamp).delete() diff --git a/src/sentry/nodestore/multi/backend.py b/src/sentry/nodestore/multi/backend.py index 6c225795b5771e..0656f744cf92a6 100644 --- a/src/sentry/nodestore/multi/backend.py +++ b/src/sentry/nodestore/multi/backend.py @@ -78,3 +78,14 @@ def delete(self, id): if should_raise: raise + + def cleanup(self, cutoff_timestamp): + should_raise = False + for backend in self.backends: + try: + backend.cleanup(cutoff_timestamp) + except Exception: + should_raise = True + + if should_raise: + raise diff --git a/src/sentry/nodestore/riak/backend.py b/src/sentry/nodestore/riak/backend.py index 2b9ad2faa21829..0ad5951c5fc072 100644 --- a/src/sentry/nodestore/riak/backend.py +++ b/src/sentry/nodestore/riak/backend.py @@ -53,3 +53,8 @@ def get_multi(self, id_list): def set(self, id, data): obj = self.bucket.new(key=id, data=data) obj.store() + + def cleanup(self, cutoff_timestamp): + # TODO(dcramer): we should either index timestamps or have this run + # a map/reduce (probably the latter) + raise NotImplementedError diff --git a/src/sentry/tasks/cleanup.py b/src/sentry/tasks/cleanup.py index 2e9e0ee37669cd..04ffbffe6c4ea7 100644 --- a/src/sentry/tasks/cleanup.py +++ b/src/sentry/tasks/cleanup.py @@ -23,12 +23,12 @@ def cleanup(days=30, project=None, chunk_size=1000, **kwargs): from django.utils import timezone + from sentry import app # TODO: TagKey and GroupTagKey need cleaned up from sentry.models import ( Group, Event, GroupCountByMinute, EventMapping, GroupTag, TagValue, ProjectCountByMinute, Alert, SearchDocument, Activity, LostPasswordHash) - from sentry.nodestore.django.models import Node GENERIC_DELETES = ( (SearchDocument, 'date_changed'), @@ -53,8 +53,12 @@ def cleanup(days=30, project=None, chunk_size=1000, **kwargs): date_added__lte=timezone.now() - datetime.timedelta(days=1) ).delete() + # TODO: we should move this into individual backends log.info("Removing old Node values") - Node.objects.filter(timestamp__lte=ts) + try: + app.nodestore.cleanup(ts) + except NotImplementedError: + log.warning("Node backend does not support cleanup operation") # Remove types which can easily be bound to project + date for model, date_col in GENERIC_DELETES: From 59a9429480512daae33e44afdf3c8b0415dbb751 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Sat, 19 Oct 2013 19:30:43 -0700 Subject: [PATCH 33/34] Correct event_id fetching for postgres --- tests/sentry/models/tests.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/sentry/models/tests.py b/tests/sentry/models/tests.py index 5fe80555bec952..45770a116b7b74 100644 --- a/tests/sentry/models/tests.py +++ b/tests/sentry/models/tests.py @@ -190,13 +190,18 @@ def test_does_transition_data_to_node(self): if connection.features.can_return_id_from_insert: r_fmt, r_params = connection.ops.return_insert_id() if r_fmt: - query_bits.append(r_fmt % 'id') + query_bits.append(r_fmt % Event._meta.pk.column) params += r_params cursor = connection.cursor() cursor.execute(' '.join(query_bits), params) - event_id = cursor.lastrowid + if connection.features.can_return_id_from_insert: + event_id = connection.ops.fetch_returned_insert_id(cursor) + else: + event_id = connection.ops.last_insert_id( + cursor, Event._meta.db_table, Event._meta.pk.column) + event = Event.objects.get(id=event_id) assert type(event.data) == NodeData assert event.data == data From 13f84571fcfd4666c18a5ce21bc351c776e888c8 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 24 Oct 2013 21:07:32 -0700 Subject: [PATCH 34/34] Allow read choice to be customized --- src/sentry/nodestore/multi/backend.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sentry/nodestore/multi/backend.py b/src/sentry/nodestore/multi/backend.py index 0656f744cf92a6..b8b971ab6422f0 100644 --- a/src/sentry/nodestore/multi/backend.py +++ b/src/sentry/nodestore/multi/backend.py @@ -25,9 +25,9 @@ class MultiNodeStorage(NodeStorage): >>> MultiNodeStorage(backends=[ >>> ('sentry.nodestore.django.backend.DjangoNodeStorage', {}), >>> ('sentry.nodestore.riak.backend.RiakNodeStorage', {}), - >>> ]) + >>> ], read_selector=random.choice) """ - def __init__(self, backends, **kwargs): + def __init__(self, backends, read_selector=random.choice, **kwargs): assert backends, "you should provide at least one backend" self.backends = [] @@ -35,15 +35,16 @@ def __init__(self, backends, **kwargs): if isinstance(backend, basestring): backend = import_string(backend) self.backends.append(backend(**backend_options)) + self.read_selector = read_selector super(MultiNodeStorage, self).__init__(**kwargs) def get(self, id): # just fetch it from a random backend, we're not aiming for consistency - backend = random.choice(self.backends) + backend = self.read_selector(self.backends) return backend.get(id) def get_multi(self, id_list): - backend = random.choice(self.backends) + backend = self.read_selector(self.backends) return backend.get_multi(id_list=id_list) def set(self, id, data):