Re: select_for_update running on db_for_read

204 views
Skip to first unread message

Ioan Alexandru Cucu

unread,
Feb 18, 2013, 10:42:30 AM2/18/13
to django-d...@googlegroups.com
The correct patch would be:
(Set _for_write on the cloned object rather than the original...)

--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -664,6 +664,7 @@ class QuerySet(object):
         # Default to false for nowait
         nowait = kwargs.pop('nowait', False)
         obj = self._clone()
+        obj._for_write = True
         obj.query.select_for_update = True
         obj.query.select_for_update_nowait = nowait
         return obj

Shai Berger

unread,
Feb 19, 2013, 2:23:29 PM2/19/13
to django-d...@googlegroups.com
Hi,

On Monday 18 February 2013, Ioan Alexandru Cucu wrote:
>
> If I'm running a select_for_update statement in a multidb environment that
> uses a read-only slave database
>
Why does this operation make sense?

Why would you select-for-update if you don't intend to update? Or does
updating on a read-only slave somehow make sense?

Thanks,
Shai.

Ioan Alexandru Cucu

unread,
Feb 20, 2013, 2:44:11 AM2/20/13
to django-d...@googlegroups.com
Hi,

Well that's what I'm saying. It doesn't make any sense.
By default, django runs the select_for_update query on the 'for read' database instead of running it on the 'for write' database.

In order to make my code not to break, I need to explicitly tell django that I want the query to run on the 'for write' database.

=> I need to write:
pages = Page.objects.select_for_update().using(router.db_for_write(Page)).all()
instead of just
pages = Page.objects.select_for_update().all()

If I don't add the 'using(router.db_for_write(Page)' I get the traceback mentioned in my initial message.

Regards,
Alex

Russell Keith-Magee

unread,
Feb 20, 2013, 2:58:36 AM2/20/13
to django-d...@googlegroups.com
Hi Ioan,

I'd need to dig into the code to be 100% certain, but what you've described seems plausible, and the fix looks like it's going in the right direction. A ticket is certainly called for.

As for the patch -- it needs tests :-)

Django's test suite has support for testing multiple-database situations; check the regressiontests/multiple_database test suite for examples of how to use it. SELECT_FOR_UPDATE has it's own test suite - modeltests/select_for_update - so the tests probably belong in there.

Yours,
Russ Magee %-)

On Mon, Feb 18, 2013 at 10:59 PM, Ioan Alexandru Cucu <alexandru...@gmail.com> wrote:
Hi,

I wanted to raise a but around this, but I thought it might be a better idea to ask first on the developer's group.

If I'm running a select_for_update statement in a multidb environment that uses a read-only slave database, I get the following traceback:

Traceback:
File "/home/kux/workspace/src/other/django/django/core/handlers/base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "/home/kux/workspace/src/other/django/django/contrib/admin/options.py" in wrapper
  366.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "/home/kux/workspace/src/other/django/django/utils/decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "/home/kux/workspace/src/other/django/django/views/decorators/cache.py" in _wrapped_view_func
  89.         response = view_func(request, *args, **kwargs)
File "/home/kux/workspace/src/other/django/django/contrib/admin/sites.py" in inner
  196.             return view(request, *args, **kwargs)
File "/home/kux/workspace/src/other/django/django/db/transaction.py" in inner
  209.                 return func(*args, **kwargs)
File "/home/kux/workspace/src/other/django-cms/cms/admin/pageadmin.py" in wrap
  154.             Page.objects.db_manager(router.db_for_write(Page))\
File "/home/kux/workspace/src/other/django/django/db/models/query.py" in exists
  562.             return self.query.has_results(using=self.db)
File "/home/kux/workspace/src/other/django/django/db/models/sql/query.py" in has_results
  441.         return bool(compiler.execute_sql(SINGLE))
File "/home/kux/workspace/src/other/django/django/db/models/sql/compiler.py" in execute_sql
  818.         cursor.execute(sql, params)
File "/home/kux/workspace/src/other/django/django/db/backends/util.py" in execute
  40.             return self.cursor.execute(sql, params)
File "/home/kux/workspace/src/other/django/django/db/backends/mysql/base.py" in execute
  114.             return self.cursor.execute(query, args)
File "/home/kux/workspace/envs/hb23/local/lib/python2.7/site-packages/MySQLdb/cursors.py" in execute
  174.             self.errorhandler(self, exc, value)
File "/home/kux/workspace/envs/hb23/local/lib/python2.7/site-packages/MySQLdb/connections.py" in defaulterrorhandler
  36.     raise errorclass, errorvalue

Exception Type: DatabaseError at /admin/cms/page/add/
Exception Value: (1290, 'The MySQL server is running with the --read-only option so it cannot execute this statement')


Looking through the source code I found that django.db.models.query.QuerySet.select_for_update doesn't set the _for_write attribute before cloning the queryset.

Is this intended behaviour?
If not, the following patch would fix the issue:

--- a/django/db/models/query.py
+++ b/django/db/models/query.py
@@ -663,6 +663,7 @@ class QuerySet(object):
         """
         # Default to false for nowait
         nowait = kwargs.pop('nowait', False)
+        self._for_write = True
         obj = self._clone()
         obj.query.select_for_update = True
         obj.query.select_for_update_nowait = nowait

Note: I'm running django 1.4.1

Regards,
Alex

--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

Ioan Alexandru Cucu

unread,
Feb 26, 2013, 5:02:08 AM2/26/13
to django-d...@googlegroups.com
Hi,


And s/but/bug/ in my initial post :)

Regards,
Alex
Reply all
Reply to author
Forward
0 new messages