Closed
Bug 659446
Opened 14 years ago
Closed 14 years ago
[prod] ISE - OperationalError
Categories
(Websites Graveyard :: markup.mozilla.org, defect)
Websites Graveyard
markup.mozilla.org
Tracking
(Not tracked)
VERIFIED
FIXED
1.0
People
(Reporter: mbrandt, Assigned: brez)
Details
I ran a jmeter script against our production environment that concurrently ran 10 threads, each making 5 complex marks.
20 of the 50 marks submitted via jmeter failed with the following ISE.
Traceback (most recent call last):
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/core/handlers/base.py", line 100, in get_response
response = callback(request, *callback_args, **callback_kwargs)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/views/decorators/http.py", line 37, in inner
return func(request, *args, **kwargs)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/markup/requests.py", line 129, in save_mark
new_mark_reference = common.save_new_mark_with_data(mark_data, request.META['REMOTE_ADDR'])
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/transaction.py", line 299, in _commit_on_success
res = func(*args, **kw)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/../ffdemo/markup/common.py", line 69, in save_new_mark_with_data
new_mark = Mark.objects.create()
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/manager.py", line 138, in create
return self.get_query_set().create(**kwargs)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/query.py", line 352, in create
obj.save(force_insert=True, using=self.db)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/base.py", line 434, in save
self.save_base(using=using, force_insert=force_insert, force_update=force_update)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/base.py", line 527, in save_base
result = manager._insert(values, return_id=update_pk, using=using)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/manager.py", line 195, in _insert
return insert_query(self.model, values, **kwargs)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/query.py", line 1479, in insert_query
return query.get_compiler(using=using).execute_sql(return_id)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/sql/compiler.py", line 783, in execute_sql
cursor = super(SQLInsertCompiler, self).execute_sql(None)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/models/sql/compiler.py", line 727, in execute_sql
cursor.execute(sql, params)
File "/data/www/python/markup.mozilla.org/markup/ffdemo/vendor/src/django/django/db/backends/mysql/base.py", line 86, in execute
return self.cursor.execute(query, args)
File "/usr/lib/python2.6/site-packages/MySQLdb/cursors.py", line 173, in execute
self.errorhandler(self, exc, value)
File "/usr/lib/python2.6/site-packages/MySQLdb/connections.py", line 36, in defaulterrorhandler
raise errorclass, errorvalue
OperationalError: (1213, 'Deadlock found when trying to get lock; try restarting transaction')
Updated•14 years ago
|
Target Milestone: --- → 1.0
Comment 1•14 years ago
|
||
Brez, this looks like the transaction is being kept open too long when a sufficient amount of writes happens at the same time.
Deadlocks (which in MySQL, if I get it right, are just transaction timeouts) aren't evil, and we can just try again but this won't fix the underlying issue (some more info: <http://dev.mysql.com/doc/refman/5.0/en/innodb-deadlocks.html>).
What we could do is take everything complicated (including and especially bcrypt) out of the transaction and instead of wrapping the entire save() function in a transaction decorator, we could just create all the data before, and then wrap the create(), reference=... and save() calls in a transaction by themselves.
Updated•14 years ago
|
Assignee: nobody → jbresnik
Yea makes sense, so move the create/save into its own method and then move the transaction decorator to that method?
Comment 3•14 years ago
|
||
(In reply to comment #2)
> Yea makes sense, so move the create/save into its own method and then move
> the transaction decorator to that method?
That works, or if django has a way in the docs to just start and stop (or wrap) a few lines of code in a transaction of their own, then just do that instead of the decorator. Essentially, do all the work you can before, except for generating the ref string, since that relies on the id from the DB. And then keep the actual transaction to as little code as possible.
Yea we can use a context_manager to wrap just the code that we want - doing that now..
Sorry just found out the hard way, context_manager approach isn't available until django 1.3 - will use method / decorator..
All set - moved the disk I/O out as well:
https://github.com/mozilla/markup/commit/438823f54bffffa5d71f20b551b7cd68f2a9f330
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 7•14 years ago
|
||
This is almost okay, except for the part where you store something on the harddrive without even trying to store to the DB first. Instead, you should probably wrap the "Create and return Mark" in a try/except/else block and if there was no exception, *then* store the raw data to the HD. Otherwise we'll end up with data on the HD that doesn't have an equivalent in the DB.
Otherwise, good job and thanks!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Updated•3 years ago
|
Product: Websites → Websites Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•