[Django] #21039: Support Postgres "CREATE INDEX CONCURRENTLY" in migrations
Django
-----------------------------+--------------------
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------
When a migration creates a new index, it's sometimes preferable to use a
concurrent index creation so it won't interfere with the operation of the
DB:
http://www.postgresql.org/docs/9.2/static/sql-createindex.html#SQL-
CREATEINDEX-CONCURRENTLY
It would be nice to have an option to enable this in migrations.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: FunkyBob | Owner: jcatalan
Type: New feature | Status: assigned
Component: Migrations | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jcatalan):
* status: new => assigned
* needs_docs: => 0
* owner: => jcatalan
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:1>
Django
Reporter: FunkyBob | Owner: jcatalan
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:2>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by andrewgodwin):
* status: assigned => new
* owner: jcatalan =>
Comment:
This won't make it into 1.7, as it's too big of a change.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:3>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by BlindHunter):
Why don't we want to always add CONCURRENTLY statement ? It's a bit slower
but allows us to execute zero downtime updates without dirty hacks like
create "fake" migration which adds indexes and add them in management
commands with "create index concurrently".
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:4>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by charettes):
@BlindHunter, concurrent index creation has many
[http://www.postgresql.org/docs/current/static/sql-createindex.html#SQL-
CREATEINDEX-CONCURRENTLY documented caveats] hence why it's not enabled by
default.
I don't think we should expect newcomers to Django/PostgreSQL to make sure
all their index have been correctly created even if their migration ran
fine. That's the reason we're planing to make it an ''opt-in'' feature and
not the default behavior, just like it's not the default in PostgreSQL
itself.
In the meanwhile I suggest you create your own subclass of
`Alter(Index|Unique)` operations and override `database_forward` to issue
the desired SQL.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:5>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by erikvanzijst):
> In the meanwhile I suggest you create your own subclass of
Alter(Index|Unique) operations and override database_forward to issue the
desired SQL.
`CREATE INDEX CONCURRENTLY` doesn't run inside a transaction. Can
`Operation` subclasses execute SQL outside the current transaction?
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:6>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by erikvanzijst):
* cc: erik.van.zijst, at, gmail (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:7>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by shaib):
#25833 is more general, but it would allow this to be written easily as
user code. I'd even consider closing this ticket as a duplicate of #25833.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:8>
Django
Reporter: FunkyBob | Owner:
Type: New feature | Status: closed
Component: Migrations | Version: master
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* status: new => closed
* resolution: => duplicate
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:9>
Django
Reporter: FunkyBob | Owner: (none)
Type: New feature | Status: new
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: closed => new
* resolution: duplicate =>
Comment:
With #25833 resolved, it's not clear to me how this is a duplicate of that
issue. #28448 is a new ticket requesting support for concurrently indexes.
I'm reopening this ticket and closing that new ticket as a duplicate of
this one.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:10>
Django
Reporter: FunkyBob | Owner: (none)
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Dan Tao):
Now that #25833 is resolved, it seems to me there are (at least) two
possible ways to approach this:
1. Add a new `CreateIndexConcurrently` operation in the
django.contrib.postgres.operations module.
2. Add a new `concurrent` argument to the constructor for the base
`AddIndex` operation, and conditionally support this argument via feature
detection based on a new attribute (maybe something like
`supports_concurrent_index_creation`) on the base schema editor class.
Database backends that don't support concurrent index creation (i.e.
everything but PostgreSQL at present) would presumably raise
`NotSupportedError` or something along those lines.
The first option seems simpler to me, at least for now, unless there is
compelling evidence that another database backend will support concurrent
index creation in the near future.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:11>
Django
Reporter: FunkyBob | Owner: (none)
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Simon Charette):
> The first option seems simpler to me, at least for now, unless there is
compelling evidence that another database backend will support concurrent
index creation in the near future.
I agree with your conclusions; there's little benefit to add a
`concurrent` option to `CreateIndex` for now. If any supported database
adds support for this option in the future it should be easy enough to
deprecate `contrib.postgres.CreateIndexConcurrently`.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:12>
Django
Reporter: FunkyBob | Owner: (none)
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Dan Tao):
* cc: Dan Tao (added)
* has_patch: 0 => 1
Comment:
I've opened a pull request to add the `CreateIndexConcurrently` operation
proposed in my previous comment:
https://github.com/django/django/pull/10901
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:13>
Django
Reporter: FunkyBob | Owner: (none)
Component: Migrations | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_docs: 0 => 1
Comment:
Please add documentation. See the
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/#patch-review-checklist patch review checklist].
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:14>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Dan Tao):
* owner: (none) => Dan Tao
* status: new => assigned
Comment:
Per [https://docs.djangoproject.com/en/dev/internals/contributing/writing-
code/submitting-patches/#claiming-tickets the guidelines on claiming
tickets], I'm assigning this to myself.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:15>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Dan Tao):
* needs_docs: 1 => 0
Comment:
I've
[https://github.com/django/django/pull/10901/commits/b98a0db0f0d33852a74c0ce53d44ddbd1af74d9b
added documentation], so I'm unsetting the **Needs documentation** flag.
Of course I am happy to add more docs or update what I've written based on
feedback.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:16>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Asif Saifuddin Auvi):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:17>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:18>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 1 => 0
Comment:
[https://github.com/django/django/pull/11594 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:19>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mads Jensen):
* needs_better_patch: 0 => 1
Comment:
Since there are still things that need work, I'm marking this as WIP.
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:20>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mads Jensen):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:21>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for checkin
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:22>
Django
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: FunkyBob | Owner: Dan Tao
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
* component: Migrations => contrib.postgres
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: FunkyBob | Owner: Dan Tao
Component: contrib.postgres | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"85ac838d9e6975130b5c55299e9d7d1222a8e289" 85ac838d]:
{{{
#!CommitTicketReference repository=""
revision="85ac838d9e6975130b5c55299e9d7d1222a8e289"
Fixed #21039 -- Added AddIndexConcurrently/RemoveIndexConcurrently
operations for PostgreSQL.
Thanks to Simon Charettes for review.
Co-Authored-By: Daniel Tao <danie...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21039#comment:25>