On Nov 25, 2010, at 7:46 AM, Russell Keith-Magee wrote: > We need to declare that the current behavior to be a bug. We can > break backwards compatibility to correct behavior that is clearly > wrong. I haven't fully thought through the consequences here, but I > think the combination of the footprint of affected cases, combined > with the side effects of having dangling connections and transactions > might almost be enough to convince me that can invoke the bug clause > to break backwards compatibility. If you (or anyone else) has any > opinions on this, I'd be interested in hearing them.
I'd definitely argue that the current behavior is a bug. In the case (not the least bit unusual) of Django applications connecting to PostgreSQL through a connection pooler (usually pg_bouncer), it's pretty common to see Idle in Transaction connections start piling up because of this problem. Thus, real-world consequences arise from this, but I'd also argue it is a bug even just considering the behavior within Django. More below... > To be clear -- as I understand it, we're talking about any code that: > > * is in a transaction managed block (i.e., between manually invoked > enter/leave_transaction_management() calls, or within the scope of a > commit_manually decorator/context manager), and > > * has a select or other manual cursor activity *after* the last > commit/rollback, but *before* the end of the transaction management > block, when there hasn't been a model save or other 'dirtying' > behavior invoked after the last commit/rollback > > At present, such code is allowed to pass, and the transaction dangles. > The proposed change would declare this situation a bug, requiring a > manual commit/rollback at the end of any database activity. That's correct, with one caveat below. My argument for this being a bug is that the behavior is indeterminate, and the most likely behavior is (in my view) surprising. Right now, the transaction will stay open until the connection closes; this will probably cause a rollback at the database, but it's not a promise, simply the way the database happens to work. This is both relying on a pretty gritty level of implementation, and doing a rollback rather than a commit is, I'd argue, surprising for the most typical cases. The caveat is that there is also a behavior change to code which uses @commit_on_success (or creates similar behavior), does not do anything to cause is_dirty to be set, modifies the database in some other way, and then *relies* on the typical rollback behavior. In the proposed fix, such transactions will commit instead. This is, however, pretty much the same as relying on uninitialized values in memory to be consistent, and I don't see any reason not to declare such code buggy. I've gotten around this by having my own version of commit_on_success that always commits rather than checking the is_dirty flag, but it would be great to have this in the core. -- -- Christophe Pettus x...@thebuild.com -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
