Re: select_for_update running on db_for_read

[Russell Keith-Magee]

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 <
alexandruioan.c...@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-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@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.
>
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: select_for_update running on db_for_read

[Ioan Alexandru Cucu]

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

On Tuesday, February 19, 2013 9:23:29 PM UTC+2, Shai Berger wrote:
>
> 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. 
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Database pooling vs. persistent connections

[Alex Gaynor]

I don't really see what point such an option would serve. This is *not a
connection pool*. Currently Django uses one open connection per thread,
this simple keeps them open between requests. That means before: you needed
O(concurrent threads) connections, now you need O(total active threads)
connections. If your database can't handle that many connections, why do
you have that many threads running, peak load would already be making it
fall over.

Alex


On Tue, Feb 19, 2013 at 9:52 PM, Eric Florenzano  wrote:

> One question: does this proposal include some way of specifying a maxiumum
> number of outstanding connections to the database from a single process?
>  Looked but didn't see it in the pull request.  In my experience, most
> PostgreSQL instances don't have a whole lot of breathing room in terms of
> the number of total outstanding connections they can handle due to low
> shared memory defaults etc. This is something that django-postgrespool does
> by way of SQLAlchemy's pool_size and max_overflow settings.
>
> --
> 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-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@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.
>
>
>



-- 
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Database pooling vs. persistent connections

[Eric Florenzano]

One question: does this proposal include some way of specifying a maxiumum 
number of outstanding connections to the database from a single process? 
 Looked but didn't see it in the pull request.  In my experience, most 
PostgreSQL instances don't have a whole lot of breathing room in terms of 
the number of total outstanding connections they can handle due to low 
shared memory defaults etc. This is something that django-postgrespool does 
by way of SQLAlchemy's pool_size and max_overflow settings.

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Django 1.4.4 and contrib/sessions/management/__init__.pyc

[Karen Tracey]

On Tue, Feb 19, 2013 at 10:12 PM, Nick Popoff  wrote:

> In the case of ./contrib/sessions/management/__init__.pyc an __init__.py
> is not provided.
>
> Is it possible compiled 2.7 .pyc files were included instead of .py files
> by mistake?
>

The .pyc files were certainly included by mistake, the tarballs should not
contain them. In the case of contrib/sessions/management, that directory
shouldn't exist in 1.4 at all, it's new in 1.5. So the corresponding .py
file isn't missing, the .pyc file is just (even more) extraneous.

Karen

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Django 1.4.4 and contrib/sessions/management/__init__.pyc

[Nick Popoff]

Ah it looks like there's already a ticket on it:

https://code.djangoproject.com/ticket/19858

On Tuesday, February 19, 2013 7:12:52 PM UTC-8, Nick Popoff wrote:
>
> Hi folks,
>
> I downloaded Django 1.4.4 this evening and after switching to it started 
> getting the following error on my CentOS 6.3 / Python 2.6 based system:
>
> ImportError: Bad magic number in 
> /usr/lib/python2.6/site-packages/django/contrib/sessions/management/__init__.pyc
>
> After confirming my md5sum matches the published signature I noticed, 
> unlike the previous version, the Django 1.4.4 source tarball includes 
> several .pyc files:
>
> $ find . -name \*.pyc
> ./conf/locale/__init__.pyc
> ./conf/locale/en/__init__.pyc
> ./conf/locale/en/formats.pyc
> ./contrib/sessions/management/__init__.pyc
>
> In the case of ./contrib/sessions/management/__init__.pyc an __init__.py 
> is not provided.
>
> Is it possible compiled 2.7 .pyc files were included instead of .py files 
> by mistake?
>
> My apologies in advance if I've misunderstood some aspect of Django 
> packaging/installation and thanks for this awesome project...
>
>

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Django 1.4.4 and contrib/sessions/management/__init__.pyc

[Nick Popoff]

Hi folks,

I downloaded Django 1.4.4 this evening and after switching to it started 
getting the following error on my CentOS 6.3 / Python 2.6 based system:

ImportError: Bad magic number in 
/usr/lib/python2.6/site-packages/django/contrib/sessions/management/__init__.pyc

After confirming my md5sum matches the published signature I noticed, 
unlike the previous version, the Django 1.4.4 source tarball includes 
several .pyc files:

$ find . -name \*.pyc
./conf/locale/__init__.pyc
./conf/locale/en/__init__.pyc
./conf/locale/en/formats.pyc
./contrib/sessions/management/__init__.pyc

In the case of ./contrib/sessions/management/__init__.pyc an __init__.py is 
not provided.

Is it possible compiled 2.7 .pyc files were included instead of .py files 
by mistake?

My apologies in advance if I've misunderstood some aspect of Django 
packaging/installation and thanks for this awesome project...

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: clarification of API backwards-compatibility policy

[Russell Keith-Magee]

On Wed, Feb 20, 2013 at 5:56 AM, Carl Meyer  wrote:

> Hi,
>
> I was just about to tell someone on IRC that Django's
> backwards-compatibility policy only applies to documented methods and
> attributes (which is how I'd always understood it), but when I actually
> went to look at the documented policy it isn't as clear as I'd hoped :/
>
> https://docs.djangoproject.com/en/dev/misc/api-stability/
>
> That says "All the public APIs – everything documented in the linked
> documents below, and all methods that don’t begin with an underscore –
> will not be moved or renamed without providing backwards-compatible
> aliases."
>
> This is a bit unclear: does it really mean _every_ method _anywhere_
> that doesn't begin with an underscore? Or does it just mean every
> non-underscore method of an otherwise-documented class, even if the
> method itself is not documented?
>
> Either way, I think it would be clearer (and more accurate to actual
> practice) if we removed all mention of underscores in this document (or
> even explicitly said that we don't generally use the underscore
> convention), and clarified that back-compat applies only to documented
> modules, classes, methods, and attributes.
>
> Thoughts?
>

+1.

If I had to try and reverse engineer why the underscore statement was
included in the first place, I'd guess it had something to do with _meta,
or with some of the more useful (but undocumented) methods on forms.
Regardless, I agree that documenting a method should be our mark of API
stability, not a naming convention.

Yours,
Russ Magee %-)

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: clarification of API backwards-compatibility policy

[Aymeric Augustin]

On 19 févr. 2013, at 22:56, Carl Meyer  wrote:
> I was just about to tell someone on IRC that Django's
> backwards-compatibility policy only applies to documented methods and
> attributes (which is how I'd always understood it), but when I actually
> went to look at the documented policy it isn't as clear as I'd hoped :/
> 
> https://docs.djangoproject.com/en/dev/misc/api-stability/

I said that on IRC recently, was called out, and then filed a ticket:
https://code.djangoproject.com/ticket/19728

> Either way, I think it would be clearer (and more accurate to actual
> practice) if we removed all mention of underscores in this document (or
> even explicitly said that we don't generally use the underscore
> convention), and clarified that back-compat applies only to documented
> modules, classes, methods, and attributes.


I agree, the docs don't reflect the current practice and should be changed.
We aren't using the underscore convention (at least, not consistently).
Even if we wanted to, that ship has sailed.

-- 
Aymeric.



-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: ANNOUNCE: Django 1.5 release candidate 2, Django 1.4.4, Django 1.3.6 (security releases)

[Carl Meyer]

Hi Nick,

On 02/19/2013 03:32 PM, Nick Phillips wrote:
> I don't recall looking at the ALLOWED_HOSTS setting before. Now that I
> do, it seems rather problematic. In particular, that host verification
> is apparently turned off while DEBUG is True or while testing.
> 
> Surely this makes it impossible to test, and makes it likely that
> misconfigurations will not be picked up until deployed to a production
> environment.
>
> Given that most setups require some customisation of settings for
> dev/staging/production/whatever environments anyway, why not leave the
> verification on at all times and allow us to ensure we get the right
> hosts in the right environments?

There was extensive back-and-forth discussion of this in writing the
patch. The issue is that in almost all cases the correct value of the
setting in local development and under test are different from the
correct value in production. So how much value is there in knowing the
tests pass, and/or it works locally? That doesn't really increase the
chances that you have it configured correctly in production. In the end,
our estimation was that requiring it to be configured in local dev and
testing would introduce quite a lot of hassle (and potentially a major
roadblock for new users), with very little gain to offset that.

Note that it isn't impossible to test, if you want to; the check is
"disabled" under test by means of setting ALLOWED_HOSTS to ['*'] for the
duration of the test run, so if you want tests to exercise validation
you can just use the override_settings decorator to temporarily change
it for a test method or test case.

Carl

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: ANNOUNCE: Django 1.5 release candidate 2, Django 1.4.4, Django 1.3.6 (security releases)

[Nick Phillips]

On Tue, 2013-02-19 at 14:50 -0600, James Bennett wrote:
> We've issued several security releases today. Details are in the blog
post:
> 
> https://www.djangoproject.com/weblog/2013/feb/19/security/
> 
> We recommend everyone carefully read this one, as it has an
> end-user-visible change requiring action beyond simply upgrading your
> Django package.
> 

I don't recall looking at the ALLOWED_HOSTS setting before. Now that I
do, it seems rather problematic. In particular, that host verification
is apparently turned off while DEBUG is True or while testing.

Surely this makes it impossible to test, and makes it likely that
misconfigurations will not be picked up until deployed to a production
environment.

Given that most setups require some customisation of settings for
dev/staging/production/whatever environments anyway, why not leave the
verification on at all times and allow us to ensure we get the right
hosts in the right environments?

What am I missing?


Cheers,


Nick
-- 
Nick Phillips / +64 3 479 4195 / nick.phill...@otago.ac.nz
# these statements are my own, not those of the University of Otago

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: clarification of API backwards-compatibility policy

[Alex Gaynor]

I agree, backwards compatibility for an API should be opt-in (via
documenting), not opt-out (via naming).

Alex


On Tue, Feb 19, 2013 at 1:56 PM, Carl Meyer  wrote:

> Hi,
>
> I was just about to tell someone on IRC that Django's
> backwards-compatibility policy only applies to documented methods and
> attributes (which is how I'd always understood it), but when I actually
> went to look at the documented policy it isn't as clear as I'd hoped :/
>
> https://docs.djangoproject.com/en/dev/misc/api-stability/
>
> That says "All the public APIs – everything documented in the linked
> documents below, and all methods that don’t begin with an underscore –
> will not be moved or renamed without providing backwards-compatible
> aliases."
>
> This is a bit unclear: does it really mean _every_ method _anywhere_
> that doesn't begin with an underscore? Or does it just mean every
> non-underscore method of an otherwise-documented class, even if the
> method itself is not documented?
>
> Either way, I think it would be clearer (and more accurate to actual
> practice) if we removed all mention of underscores in this document (or
> even explicitly said that we don't generally use the underscore
> convention), and clarified that back-compat applies only to documented
> modules, classes, methods, and attributes.
>
> Thoughts?
>
> Carl
>
> --
> 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-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@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.
>
>
>


-- 
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

clarification of API backwards-compatibility policy

[Carl Meyer]

Hi,

I was just about to tell someone on IRC that Django's
backwards-compatibility policy only applies to documented methods and
attributes (which is how I'd always understood it), but when I actually
went to look at the documented policy it isn't as clear as I'd hoped :/

https://docs.djangoproject.com/en/dev/misc/api-stability/

That says "All the public APIs – everything documented in the linked
documents below, and all methods that don’t begin with an underscore –
will not be moved or renamed without providing backwards-compatible
aliases."

This is a bit unclear: does it really mean _every_ method _anywhere_
that doesn't begin with an underscore? Or does it just mean every
non-underscore method of an otherwise-documented class, even if the
method itself is not documented?

Either way, I think it would be clearer (and more accurate to actual
practice) if we removed all mention of underscores in this document (or
even explicitly said that we don't generally use the underscore
convention), and clarified that back-compat applies only to documented
modules, classes, methods, and attributes.

Thoughts?

Carl

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

ANNOUNCE: Django 1.5 release candidate 2, Django 1.4.4, Django 1.3.6 (security releases)

[James Bennett]

We've issued several security releases today. Details are in the blog post:

https://www.djangoproject.com/weblog/2013/feb/19/security/

We recommend everyone carefully read this one, as it has an
end-user-visible change requiring action beyond simply upgrading your
Django package.

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: select_for_update running on db_for_read

[Shai Berger]

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.

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Database pooling vs. persistent connections

[Carl Meyer]

On 02/19/2013 03:04 AM, Anssi Kääriäinen wrote:
> I hope this discussion is about what to do at request finish/start
> time.
> 
> I am very strongly opposed to anything where Django suddenly changes
> connections underneath you. At request finish/start this is OK (you
> should expect new connections then anyways), but otherwise if you get
> broken connection, it isn't Django's business to swap the connection
> underneath you.

I agree completely. I assumed we were talking about closing the
connection at request-finished time, which would cause a new one to be
opened on the next request.

> I think a good approach would be to mark the connection potentially
> broken on errors in queries, and then in request_finished check for
> this potentially broken flag. If flag set, then and only then run
> ping() / select 1. So, this is a slight modification of no. 3 where
> one can mark the connection potentially broken liberally, but the
> connection is swapped only when the ping fails, and only in
> request_finished. For most requests there should be no overhead as
> errors in queries are rare.

I like this idea. The ping is a minimal cost if you only pay it when
there is a query error during the request, and it's more reliable than
trying to parse error messages to figure out which ones require closing
the connection.

I think we should exclude some classes of errors that we already
distinguish and know they don't mean the connection is broken (e.g.
IntegrityError). If some codebase makes heavy use of algorithms like
get_or_create that rely on a correctly-handled IntegrityError, they
shouldn't need to pay the cost of the ping more frequently.

Carl

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Database pooling vs. persistent connections

[Anssi Kääriäinen]

On 19 helmi, 12:20, Aymeric Augustin
 wrote:
> On 19 févr. 2013, at 11:04, Anssi Kääriäinen  wrote:
>
> > Maybe we need another setting for what to do in request.start. It does
> > seem somewhat likely that users could do SET SEARCH_PATH in middleware
> > to support multitenant setups for example, and expect that set to go
> > away when connection is closed after the request. Any other SET is a
> > likely candidate for problems in PostgreSQL, and I am sure other DBs
> > have their equivalent of session state, too. (In this case doing RESET
> > ALL; run connection setup again is the right thing to do in
> > PostgreSQL).
>
> > It would be nice to support this use case, but just documenting this
> > change clearly in the release notes, and point out that if you have
> > such requirements, then set max_age to 0. More features can always be
> > added later on.
>
> Yes, the short term solution in this case is to set max_age to 0.
>
> I think the long term solution is to modify such middleware to revert its
> effects in process_response() — or reset everything in process_request().
>
> I agree with Carl that I should document this thoroughly.

Yeah, just doing the reset manually in middleware should be good
enough.

> > I think a good approach would be to mark the connection potentially
> > broken on errors in queries, and then in request_finished check for
> > this potentially broken flag. If flag set, then and only then run
> > ping() / select 1. So, this is a slight modification of no. 3 where
> > one can mark the connection potentially broken liberally, but the
> > connection is swapped only when the ping fails, and only in
> > request_finished. For most requests there should be no overhead as
> > errors in queries are rare.
>
> This is an interesting idea. Django already catches database exceptions in
> execute() to re-raise them. We could simply set the flag there.
>
> > BTW the remark above in Aymeric's post that persistent connections
> > can't make things worse: I don't believe this. Persistent connections
> > will keep the broken connection from request to request, and at least
> > on PostgreSQL a broken connection is correctly closed in request
> > finish.
>
> https://code.djangoproject.com/ticket/15802says it isn't — although I haven't
> confirmed that this bug still exists.

I just tried this and broken connections are closed correctly at least
in master. First paragraph of comment 
https://code.djangoproject.com/ticket/15802#comment:20
says this is already fixed in 1.4. The open part of the ticket seems
to be about 1.3 (which is not going to be fixed anyways) or automatic
connection swapping (which is dangerous). So maybe it is time to
reclose the ticket?

 - Anssi

-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Database pooling vs. persistent connections

[Aymeric Augustin]

On 19 févr. 2013, at 11:04, Anssi Kääriäinen  wrote:

> Maybe we need another setting for what to do in request.start. It does
> seem somewhat likely that users could do SET SEARCH_PATH in middleware
> to support multitenant setups for example, and expect that set to go
> away when connection is closed after the request. Any other SET is a
> likely candidate for problems in PostgreSQL, and I am sure other DBs
> have their equivalent of session state, too. (In this case doing RESET
> ALL; run connection setup again is the right thing to do in
> PostgreSQL).
> 
> It would be nice to support this use case, but just documenting this
> change clearly in the release notes, and point out that if you have
> such requirements, then set max_age to 0. More features can always be
> added later on.

Yes, the short term solution in this case is to set max_age to 0.

I think the long term solution is to modify such middleware to revert its
effects in process_response() — or reset everything in process_request().

I agree with Carl that I should document this thoroughly.


> (…)


> I hope this discussion is about what to do at request finish/start
> time.

You're right, it's very important not to fiddle with connections during
request processing.

> I am very strongly opposed to anything where Django suddenly changes
> connections underneath you. At request finish/start this is OK (you
> should expect new connections then anyways), but otherwise if you get
> broken connection, it isn't Django's business to swap the connection
> underneath you. There is a reasonable expectation that while you are
> using single connections[alias] in a script for example, you can
> expect the underlying connection to be the same for the whole time.
> Otherwise SET somevar in postgresql could break for example.

Even worse, you could break transactional integrity! See this comment:
https://code.djangoproject.com/ticket/15119#comment:9

> I think a good approach would be to mark the connection potentially
> broken on errors in queries, and then in request_finished check for
> this potentially broken flag. If flag set, then and only then run
> ping() / select 1. So, this is a slight modification of no. 3 where
> one can mark the connection potentially broken liberally, but the
> connection is swapped only when the ping fails, and only in
> request_finished. For most requests there should be no overhead as
> errors in queries are rare.

This is an interesting idea. Django already catches database exceptions in
execute() to re-raise them. We could simply set the flag there.

> BTW the remark above in Aymeric's post that persistent connections
> can't make things worse: I don't believe this. Persistent connections
> will keep the broken connection from request to request, and at least
> on PostgreSQL a broken connection is correctly closed in request
> finish.

https://code.djangoproject.com/ticket/15802 says it isn't — although I haven't
confirmed that this bug still exists.

-- 
Aymeric.



-- 
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-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@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.

Re: Database pooling vs. persistent connections

[Anssi Kääriäinen]

On 19 helmi, 02:31, Carl Meyer  wrote:
> On 02/18/2013 02:27 PM, Aymeric Augustin wrote:
>
> > Problem #1: Is it worth re-executing the connection setup at the beginning 
> > of
> > every request?
>
> > The connection setup varies widely between backends:
> > - SQLite: none
> > - 
> > PostgreSQL:https://github.com/django/django/blob/master/django/db/backends/postg...
> > - 
> > MySQL:https://github.com/django/django/blob/master/django/db/backends/mysql...
> > - 
> > Oracle:https://github.com/django/django/blob/master/django/db/backends/oracl...
>
> > The current version of the patch repeats it every time. In theory, this 
> > isn't
> > necessary. Doing it only once would be more simple.
>
> > It could be backwards incompatible, for instance, if a developer changes the
> > connection's time zone. But we can document to set CONN_MAX_AGE = 0 to 
> > restore
> > the previous behavior in such cases.
>
> It seems silly to re-run queries per-request that were only ever
> intended to be per-connection setup. So I favor the latter. I think this
> change will require prominent discussion in the
> potentially-backwards-incompatible section of the release notes regardless.
>
> (This could form an argument for keeping the built-in default 0, and
> just setting it to a higher number in the project template. But I don't
> like polluting the default project template, and I think a majority of
> existing upgrading projects will benefit from this without problems, so
> I don't actually want to do that.)

Maybe we need another setting for what to do in request.start. It does
seem somewhat likely that users could do SET SEARCH_PATH in middleware
to support multitenant setups for example, and expect that set to go
away when connection is closed after the request. Any other SET is a
likely candidate for problems in PostgreSQL, and I am sure other DBs
have their equivalent of session state, too. (In this case doing RESET
ALL; run connection setup again is the right thing to do in
PostgreSQL).

It would be nice to support this use case, but just documenting this
change clearly in the release notes, and point out that if you have
such requirements, then set max_age to 0. More features can always be
added later on.

> > Problem #2: How can Django handle situations where the connection to the
> > database is lost?
>
> > Currently, with MySQL, Django pings the database server every time it 
> > creates
> > a cursor, and reconnects if that fails. This isn't a good practice and this
> > behavior should be removed:https://code.djangoproject.com/ticket/15119
>
> > Other backends don't have an equivalent behavior. If a connection was 
> > opened,
> > Django assume it works. Worse, if the connection breaks, Django fails to 
> > close
> > it, and keeps the broken connection instead of opening a new one:
> >https://code.djangoproject.com/ticket/15802
>
> > Thus, persistent connections can't make things worse :) but it'd be nice to
> > improve Django in this area, consistently across all backends.
>
> > I have considered four possibilities:
>
> > (1) Do nothing. At worst, the connection will be closed after max_age and 
> > then
> >     reopened. The worker will return 500s in the meantime. This is the 
> > current
> >     implementation.
>
> > (2) "Ping" the connection at the beginning of each request, and if it 
> > doesn't
> >     work, re-open it. As explained above, this isn't a good practice. Note
> >     that if Django repeats the connection setup before each request, it can
> >     take this opportunity to check that the connection works and reconnect
> >     otherwise. But I'm not convinced I should keep this behavior.
>
> > (3) Catch database exceptions in all appropriate places, and if the 
> > exception
> >     says that the connection is broken, reconnect. In theory this is the 
> > best
> >     solution, but it's complicated to implement. I haven't found a 
> > conclusive
> >     way to identify error conditions that warrant a reconnection.
>
> > (4) Close all database connections when a request returns a 500. It's a bad
> >     idea because it ties the HTTP and database layers. It could also hide
> >     problems.
>
> I'd be inclined to go for (1), with the intent of moving gradually
> towards (3) as specific detectable error conditions that happen in real
> life and do warrant closing the connection and opening a new one are
> brought to our attention. Unfortunately handling those cases is likely
> to require parsing error messages, as pretty much anything related to
> DBAPI drivers and error conditions does :/
>
> I tried to dig for the origins of the current MySQL behavior to see if
> that would illuminate such a case, but that code goes way back into the
> mists of ancient history (specifically, merger of the magic-removal
> branch), beyond which the gaze of "git annotate" cannot penetrate.
>
> Option (4) is very bad IMO, and (2) is not much better.

I hope this discussion is about what to do at request