Opened 3 years ago

Closed 3 years ago

Last modified 22 months ago

#28062 closed Bug (fixed)

Using QuerySet.iterator() with pgBouncer leads to nonexistent cursor errors

Reported by: Sergey Fursov Owned by: François Freitag
Component: Database layer (models, ORM) Version: 1.11
Severity: Release blocker Keywords:
Cc: josh.smeaton@…, mail@…, lcannon@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi!

In our application stack we connect from our web/background workers to postgres database through dedicated pgBouncer service.
pgBouncer is running in transaction pooling mode and django connections work in autocommit mode.

After upgrading to 1.11 version with new server-side cursors for queryset iterators, many of our DB requests were started failing with "Cursor _django_curs_<id> does not exist'.

According to pgBouncer docs (https://wiki.postgresql.org/wiki/PgBouncer), it doesn't support using WITH HOLD cursors with transaction pooling mode. But it doesn't seems reasonable to run pgBouncer in session pooling mode. We can just switch to direct persistent connections to postgres DB, but in this case we have to increase minimum number of DB connection to number of our workers (from 5 to ~30-40) to prevent blocking requests and this will violate recommendation to have (core_count * 2) + effective_spindle_count max connections to DB (https://wiki.postgresql.org/wiki/Number_Of_Database_Connections).

Introducing server-side cursors seems like breaking change in our case (and i guess many django apps use pgBouncer behind Postgres DB)
So is there some recommendations how efficiently work with postgres DB in django 1.11?

Looks like another possible option is wrapping all iterator() calls in their own transactions, but django internally use cursors in several places (e.g. in ModelChoiceIterator).

I think this possible problem should be documented in some way, and it would be great to describe some options for projects to migrate to new version.

Thanks,
Sergey

Change History (19)

comment:1 Changed 3 years ago by Tim Graham

Severity: NormalRelease blocker
Summary: Using iterators behind pgBouncer leads to non-existent cursor errosUsing QuerySet.iterator() with pgBouncer leads to nonexistent cursor errors
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 Changed 3 years ago by Josh Smeaton

Cc: josh.smeaton@… added
Severity: Release blockerNormal
Summary: Using QuerySet.iterator() with pgBouncer leads to nonexistent cursor errorsUsing iterators behind pgBouncer leads to non-existent cursor erros

I don't think documentation is a good enough solution for this. Lots of people use pgbouncer, and any of them using .iterator() are about to start experiencing crashes with what was perfectly working code. It's not fair to ask people to change code or configuration for what should have been a transparent performance (memory) improvement.

I think our options are limited, but I'd be keen to hear from others how we should approach this particular bug.

  • We could revert the change, and revisit for Django 2.0.
  • We could also add an argument to .iterator() that allows users to opt-in to the new server side cursors. We'd need to document the limitations, and suggest ideal scenarios (use transaction.atomic, or .using('non_pgbouncer_transaction_connection')).

For your particular setup, I'd maybe recommend running a second pgbouncer in session mode (or not use pgbouncer), and for queries that use .iterator(), using that specific connection. As long as iterator() isn't used in the majority of your requests, the number of connections shouldn't spike too high.

comment:3 Changed 3 years ago by Josh Smeaton

Severity: NormalRelease blocker
Summary: Using iterators behind pgBouncer leads to non-existent cursor errosUsing QuerySet.iterator() with pgBouncer leads to nonexistent cursor errors

comment:4 Changed 3 years ago by Sergey Fursov

Hi Josh,

For your particular setup, I'd maybe recommend running a second pgbouncer in session mode (or not use pgbouncer), and for queries that use .iterator(), using that specific connection. As long as iterator() isn't used in the majority of your requests, the number of connections shouldn't spike too high.

This is reasonable suggestion, thanks

We could also add an argument to .iterator() that allows users to opt-in to the new server side cursors. We'd need to document the limitations, and suggest ideal scenarios (use transaction.atomic, or .using('non_pgbouncer_transaction_connection')).

This sound like the best option for me (at least compared to reverting changes)

Another my point was the fact, that django use iterator internally in ModelChoiceField for optimizing iteration over queryset and in several serializer methods.
For my particular case it would be enough to add additional ModelChoiceIterator to override default ModelChoiceField.iterator field (we don't use DB serialization much, so i don't know, does serialization part have big impact on production using). But i think this point should be considered while working on this issue, too, because it might break apps in production.

comment:5 Changed 3 years ago by François Freitag

Cc: mail@… added

comment:6 in reply to:  2 Changed 3 years ago by Florian Apolloner

Replying to Josh Smeaton:

  • We could also add an argument to .iterator() that allows users to opt-in to the new server side cursors. We'd need to document the limitations, and suggest ideal scenarios (use transaction.atomic, or .using('non_pgbouncer_transaction_connection')).

Opting in seems like a reasonable thing to do, though given the fact that it will generally not work for anyone using pgbouncer in a specific mode, it might make more sense if we'd also provide a switch to turn this off/on globally (database options).

comment:7 Changed 3 years ago by holvianssi

Opt-in is a good solution for end user code, but then library code can't opt-in, as library code can't know if it's running in pgBouncer setup or not. A database setting would get rid of this problem, but if there are cases where you want to use cursors for some cases, but not all of the cases, then you are screwed. The complex solution is to add both the opt-in flag and database settings. The default for the opt-in flag is None, meaning use the default from database settings. True forces this feature on, and False forces the feature off. Now, this covers all cases, but might create a situation where it's not easy for users to understand when exactly cursors are used.

I'm slightly favouring the opt-in + database setting approach, next would be opt-in without database setting, and last database setting without opt-in. All of those solutions are fine for me, but I don't like the idea of removing the feature.

comment:8 Changed 3 years ago by François Freitag

Owner: changed from nobody to François Freitag
Status: newassigned

comment:9 Changed 3 years ago by François Freitag

Has patch: set

PR

Working on this issue, I came to the conclusions that None is not necessary and I prefer the simpler scheme where:

  • the use of server-side cursors is disabled for a connection if the setting is False.
  • when the setting is True, server-side cursors are used by default and users and library code can opt-out specifying chunked_fetch=False.

I believe auto opt-in server-side cursors is acceptable, since it's a memory improvement in most case. The issue here is that server-side cursors cannot be used with transaction pooling, which is global to a connection. Setting ENABLE_SERVER_SIDE_CURSORS to False for the whole connection makes sense and should disable the use of server-side cursors regardless of the chunked_fetch keyword argument.

For use cases where server-side cursors are not desirable, the user / library-code can now turn them off explicitly using chunked_fetch=False.

I think this solution is simpler and covers the use cases I can think of. Am I missing something?

comment:10 Changed 3 years ago by Lachlan Cannon

Cc: lcannon@… added

comment:11 Changed 3 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:12 Changed 3 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In 88336fdb:

Fixed #28062 -- Added a setting to disable server-side cursors on PostgreSQL.

When a connection pooler is set up in transaction pooling mode, queries
relying on server-side cursors fail. The DISABLE_SERVER_SIDE_CURSORS
setting in DATABASES disables server-side cursors for this use case.

comment:13 Changed 3 years ago by Tim Graham <timograham@…>

In 6a26242:

[1.11.x] Fixed #28062 -- Added a setting to disable server-side cursors on PostgreSQL.

When a connection pooler is set up in transaction pooling mode, queries
relying on server-side cursors fail. The DISABLE_SERVER_SIDE_CURSORS
setting in DATABASES disables server-side cursors for this use case.

Backport of 88336fdbb5e101fa25825b737169c0d6af2faa93 from master

comment:14 Changed 2 years ago by Luke Plant

There really needs to be a note about this in the 1.11 release notes, as it's a breaking change that affects a pretty common set up. There is a note only in the 1.11.1 notes, which is not where people look.

comment:15 Changed 2 years ago by Tim Graham

PR for an addition to the 1.11 release notes.

comment:16 Changed 2 years ago by GitHub <noreply@…>

In 2919a08c:

Refs #28062 -- Doc'd PostgreSQL server-side cursors as a backwards incompatible change.

comment:17 Changed 2 years ago by Tim Graham <timograham@…>

In 2e7b1e32:

[2.0.x] Refs #28062 -- Doc'd PostgreSQL server-side cursors as a backwards incompatible change.

Backport of 2919a08c20d5ae48e381d6bd251d3b0d400d47d9 from master

comment:18 Changed 2 years ago by Tim Graham <timograham@…>

In 0037cd1f:

[1.11.x] Refs #28062 -- Doc'd PostgreSQL server-side cursors as a backwards incompatible change.

Backport of 2919a08c20d5ae48e381d6bd251d3b0d400d47d9 from master

comment:19 Changed 22 months ago by José Lourenço

I am running into what seems to be the same problem, however I'm not using pgBouncer.

I'm connecting directly to a redshift database and I get this error using either the django.db.backends.postgresql_psycopg2 or the django_redshift_backend engines.

'redshift': {
	'NAME': 'dbname',
	...
	'ENGINE': 'django.db.backends.postgresql_psycopg2',
	'HOST': 'xxxxxxxxxx.redshift.amazonaws.com',
	'PORT': 5439,
 }

I've enabled DEBUG logging and I can see Django creating a server side cursor by using a DECLARE WITH HOLD statement. It then issues two queries and the second fails with a cursor "_django_curs_XXXXXXX" does not exist error message. Setting DISABLE_SERVER_SIDE_CURSORS=true seems to solve the issue.

According the Django documentation, this happens due to transaction pooling resulting in Django using a different connection for which the named cursor does not exist the second time it issues the query. However I can't understand why this is happening since afaik I'm not using pgBouncer nor any other middleware implementing transaction pooling.

Can you help me understand why this is happening?

Note: See TracTickets for help on using tickets.
Back to Top