Summary Re: Migrations: A bug or a feature needing documentation?
On Monday, August 5, 2019 at 4:55:24 PM UTC-5, Barry Johnson wrote: > > [ TL;DR: A migration may use a “replaces” list pointing to migrations that > don’t actually exist. This undocumented technique cleanly solves a > recurring difficult migration problem. We seek consensus on whether this > should become a documented feature or that it is an unexpected side effect > subject to change in the future. ] > Earlier this week, I posted a question about an undocumented behavior in the migration system, wondering if it should be formally documented and supported. The use case relates to migration of a database from one branch of a project to another when there were different migrations created in both branches. I believe this is a use case that the migration system was not (and is not) intended to solve, yet it seems to be a real-world problem. Summary of responses: After discussion, I think that -our- future approach will be more procedural: We'll deliberately create an empty migration *just before* forking a parallel branch that may require migrations, and will eventually require migrating tenants from that branch back to our mainline code. We will then use that empty migration as a placeholder, and if necessary create one or more migrations that will replace the empty step to handle any parallel schema changes. It might be wise to document this practice as a suggestion for others faced with this use case. We will still have a case where multiple migrations would each indicate that they replace one of those empty migrations, perhaps caused by multiple schema changes being applied to the "patch" fork. We've seen that behavior create apparent duplicate entries in the django_migrations database table. But the migration system loads that table into a set containing (app, name) tuples, so the duplicate entries aren't hurting anything at present. Furthermore, there is nothing that deliberately ties the list of previously applied migrations (that set of (app, name) tuples) back to migrations that exist. Entries in the table for migrations that no longer exist are a side effect of the current squashing logic. They can eventually cause the django_migrations table to contain many more rows than are necessary; perhaps it may be useful to have a migration tool or operation that clears out those obsolete records. Or, as Markus has suggested, it may be wise to have the squashing process clean up after itself by removing "replaced" entries instead of leaving them behind. I'll work up a documentation change PR that people can review and accept, modify or throw away. Thank you! baj - Barry Johnson -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/b7d694aa-735f-429f-aae6-d0b948925d27%40googlegroups.com.
Re: Migrations: A bug or a feature needing documentation?
On Wednesday, August 7, 2019 at 9:22:27 PM UTC-5, Markus Holtermann wrote: > > Let's look at the last question first, regarding duplicate entries in the > django_migrations table: Yes, this is to be a bug. At least how it's > currently used. > Agreed. > Let's say you have migration foo.0001_initial and apply it. You then have > (foo, > 0001_initial) in the django_migrations table. You now create migration > foo.0002_something and also add a squashed migration > foo.0001_initial_squashed_0002_something. When you now run migrate, > Django will apply foo.0002_something and your database will have (foo, > 0001_initial), (foo, 0002_something) as well as (foo, > 0001_initial_squashed_0002_something). > So far so good. That's all as expected. If you now remove foo.0001_initial > and foo.0002_something from your filesystem and remove the replaces > section in foo.0001_initial_squashed_0002_something it is as if Django > never new about foo.0001_initial or foo.0002_something. You can add new > migrations, everything works the way it should. However, if you were to add > e.g. foo.0002_something again, Django would treat it as already applied, > despite it being somewhere later in your migration graph. > At this point, I don't think this is the intended behavior. That said, I'm > inclined to say that applying a squashed migration should "unrecord" all > migrations it replaces. I've not yet thought too much about the "fallout" > (backwards compatibility, > rollback of migrations, ...). But at least with regards to migrating > forwards, this seems to be the right behavior. > To date, the migration system has been remarkably tolerant of spurious entries in the django_migrations table, and the squashing process (to date) does indeed leave breadcrumbs of since-deleted migrations behind. Agree completely with your point about adding a subsequent migration with a name that exactly matches a previously created migration that has been removed. For all practical purposes, the migration names should be unique over time, or the developer should ensure that the second incarnation of that name is a functional replacement. We have, in the past, replaced the contents of the "0001_initial" migration in most of our apps, but it is incumbent on us to make sure that's correct. Regarding your second point around "replaces" and merging migrations: I > think this will lead to inconsistencies in your migration order, thus > potentially causing trouble down the line. [...] I suspect that two data > migrations could easily conflict or result in inconsistent data if applied > in the wrong order. For example, one data migration adding new records to a > table, and another one ensuring that all values in a column are in upper > case. If you apply both migrations in that order (insert and then ensure > uppercase) you can be certain that all values will be uppercase. If you, > however, first ensure uppercase and then insert additional values, you need > to make sure that the data in the second migration is properly formatted. > Oh, certainly agree, Markus, about the dangers of replacing migrations. That's true across the board, even in the documented use cases. The replacement steps MUST properly account for elidable data migrations. In the normal use case (the typical migration squashing process), the replacement step truly replaces the original step, and is executed at that original spot within the dependency graph. (It can seem as if that replacement is moved "back in time", as if history were rewritten. That means that any migrations subsequent to the original step will remain subsequent to the replacement, which works. In the use case where the original does not exist, the replacement step remains where it is in the dependency graph. It must, because there's nowhere earlier that it could be placed. Could this introduce an inconsistency, especially with data migrations? Yes, if the developer is not careful. Using your example of an insertion of lowercase data followed by a conversion to uppercase data: in one branch the insertion happens first; in a different branch (where the insertion is run as a replacement step) they could be run in a different order. That would indeed appear to be one of the dangers of moving a database from one branch of code to another, and that (as things are today) the developer must understand the migration paths. Hmmm. Keeping all this in mind, I'm beginning to believe there is a procedural method to solve the parallel-development problem. Just before creating a long-lived branch (such as as production release that may get patches), create an empty migration step in all apps. Later, if patch migrations had become necessary in that branch, the mainline trunk can "replace" the empty step with an equivalent migration that replaces it. At least, that will give us an order of execution that more closely resembles what the
Re: Migrations: A bug or a feature needing documentation?
On Wednesday, August 7, 2019 at 2:17:45 PM UTC-5, Adam Johnson wrote: > > Some questions: Have you looked into the migration framework internals? > Are there any comments around "replaces" that indicate this behaviour? And > is there maybe a way of hooking into the migration planner, or adding the > ability, to support your use case without relying on the current > bug-feature? > [Looks quick...] Interesting! There are two subtle points in the source that show the author intentionally planned for this case, both in a method named "remove_replaced_nodes()" that, ahem, removes the replaced migration nodes from the dependency graph if the conditions are right for replacement. The docstring for that method begins with the sentence "Remove each of the 'replaced' nodes (when they exist)." And within the main loop of that method, the code does a "pop" to remove the replaced node, deliberately not failing if that replaced node does not appear in the graph: replaced_node = self.node_map.pop(replaced_key, None) if replaced_node: The purpose of this particular method is two-fold: It removes the replaced node, yes, but just as importantly it rewires the dependencies to and of this replaced node. Any children of the replaced node now become children of the replacement, and any parents that point at the replaced now become parents of the replacement. So deliberate use of this approach does require careful understanding of the dependency tree (which really isn't a surprise). Interesting. -- You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group. To unsubscribe from this group and stop receiving emails from it, send an email to django-developers+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/b6e576b9-ebbd-4263-8dd3-ae6ec79ced54%40googlegroups.com.
Re: Migrations: A bug or a feature needing documentation?
Hi Barry, TL;DR: I think this is a bug and can lead to inconsistencies in other project setups than yours. Let's look at the last question first, regarding duplicate entries in the django_migrations table: Yes, this is to be a bug. At least how it's currently used. Let's say you have migration foo.0001_initial and apply it. You then have (foo, 0001_initial) in the django_migrations table. You now create migration foo.0002_something and also add a squashed migration foo.0001_initial_squashed_0002_something. When you now run migrate, Django will apply foo.0002_something and your database will have (foo, 0001_initial), (foo, 0002_something) as well as (foo, 0001_initial_squashed_0002_something). So far so good. That's all as expected. If you now remove foo.0001_initial and foo.0002_something from your filesystem and remove the replaces section in foo.0001_initial_squashed_0002_something it is as if Django never new about foo.0001_initial or foo.0002_something. You can add new migrations, everything works the way it should. However, if you were to add e.g. foo.0002_something again, Django would treat it as already applied, despite it being somewhere later in your migration graph. At this point, I don't think this is the intended behavior. That said, I'm inclined to say that applying a squashed migration should "unrecord" all migrations it replaces. I've not yet thought too much about the "fallout" (backwards compatibility, rollback of migrations, ...). But at least with regards to migrating forwards, this seems to be the right behavior. Regarding your second point around "replaces" and merging migrations: I think this will lead to inconsistencies in your migration order, thus potentially causing trouble down the line. I'm yet to think of an example. For now I don't see us to change the behavior, but I would definitely *not* rely on it. I suspect that two data migrations could easily conflict or result in inconsistent data if applied in the wrong order. For example, one data migration adding new records to a table, and another one ensuring that all values in a column are in upper case. If you apply both migrations in that order (insert and then ensure uppercase) you can be certain that all values will be uppercase. If you, however, first ensure uppercase and then insert additional values, you need to make sure that the data in the second migration is properly formatted. Cheers, Markus On Tue, Aug 6, 2019, at 7:55 AM, Johnson, Barry wrote: > > [ TL;DR: A migration may use a “replaces” list pointing to migrations > that don’t actually exist. This undocumented technique cleanly solves a > recurring difficult migration problem. We seek consensus on whether > this should become a documented feature or that it is an unexpected > side effect subject to change in the future. ] > > > > We have found an undocumented behavior in the migration system that > gracefully solves the troublesome problem of merging migrations created > in parallel development branches. If this behavior should survive, > we’ll enter a documentation ticket – but if it’s considered a bug, > we’ll need to stay away from it and fall back to the more difficult > manual editing approaches we’ve used in the past. > > > The Use Case > > -- > > We’re rapidly developing a large multi-tenant application (hundreds of > ORM models, thousands of migrations and hundreds of thousands of lines > of code so far, with quite a bit of work remaining) punctuated by > periodic production releases. We create a source code branch from our > mainline development trunk for each production release, just in case we > must rapidly issue patches to those production releases. On rare > occasions, we’ve had to make a schema change (such as adding a new > field) as a patch to a production release, and make a parallel schema > change in the mainline development trunk. > > > Of course, this normally causes a migration failure when migrating a > production tenant from the patch release up to a later version of the > mainline release – since the mainline release has a subsequent > migration that adds the same field. We’ve solved this in the past by > manually rearranging the dependency order of the mainline trunk > migrations (moving the replacement step before other new migrations for > this later release), and fiddling with the contents of the > django_migrations table to make it look like that mainline step has > already been run before running the migrations. We’re unhappy with that > approach – it’s both time consuming and error prone. > > > This problem is similar to, but not identical to, that of squashing > migrations. > > > (And yes, we do periodically squash our migrations. We have about 600 > migration steps at the moment, left over from more than 2,000 > originally created. We’ve got another round of squashing coming up soon > that should take us to less than 100 migrations –
Re: Migrations: A bug or a feature needing documentation?
Hi Barry, I don't have a very strong opinion here, but replying with some questions, and to bump the thread. I think this smells more like a bug than a feature to me. I worry that if you depend on it, it could easily get refactored away in a future version of Django. If we were to document it as feature, it would also need extra tests in the test suite to ensure this regression doesn't happen. Some questions: Have you looked into the migration framework internals? Are there any comments around "replaces" that indicate this behaviour? And is there maybe a way of hooking into the migration planner, or adding the ability, to support your use case without relying on the current bug-feature? Thanks, Adam On Mon, 5 Aug 2019 at 19:36, Johnson, Barry wrote: > [ TL;DR: A migration may use a “replaces” list pointing to migrations that > don’t actually exist. This undocumented technique cleanly solves a > recurring difficult migration problem. We seek consensus on whether this > should become a documented feature or that it is an unexpected side effect > subject to change in the future. ] > > > > > > We have found an undocumented behavior in the migration system that > gracefully solves the troublesome problem of merging migrations created in > parallel development branches. If this behavior should survive, we’ll > enter a documentation ticket – but if it’s considered a bug, we’ll need to > stay away from it and fall back to the more difficult manual editing > approaches we’ve used in the past. > > > > The Use Case > > -- > > We’re rapidly developing a large multi-tenant application (hundreds of ORM > models, thousands of migrations and hundreds of thousands of lines of code > so far, with quite a bit of work remaining) punctuated by periodic > production releases. We create a source code branch from our mainline > development trunk for each production release, just in case we must rapidly > issue patches to those production releases. On rare occasions, we’ve had > to make a schema change (such as adding a new field) as a patch to a > production release, and make a parallel schema change in the mainline > development trunk. > > > > Of course, this normally causes a migration failure when migrating a > production tenant from the patch release up to a later version of the > mainline release – since the mainline release has a subsequent migration > that adds the same field. We’ve solved this in the past by manually > rearranging the dependency order of the mainline trunk migrations (moving > the replacement step before other new migrations for this later release), > and fiddling with the contents of the django_migrations table to make it > look like that mainline step has already been run before running the > migrations. We’re unhappy with that approach – it’s both time consuming > and error prone. > > > > This problem is similar to, but not identical to, that of squashing > migrations. > > > > (And yes, we do periodically squash our migrations. We have about 600 > migration steps at the moment, left over from more than 2,000 originally > created. We’ve got another round of squashing coming up soon that should > take us to less than 100 migrations – but we have more than a dozen > developers adding more migrations every week.) > > > > The Discovery > > --- > > Through trial and error, we found that our mainline migration step may > declare itself as a replacement for the patch step (using the “replaces” > attribute) – even if the patch migration itself doesn’t exist in the list > of mainline migrations. > > > > And if we do this, the migration engine simply works as hoped and our > problem vanishes. It’s absolutely wonderful; simple to implement and > effective. We love it. New tenants run only the replacement step; tenants > migrating from the patch release to the trunk release merely record the > replacement step as having been completed without actually executing it; > development tenants that never saw the original patch step simply record > both the patch step and the replacement as having been completed. It’s > great. > > > > The Worry > > -- > > This approach seems undocumented in three different ways: > > > > * The replacement migration is pointing at an original migration that > doesn’t exist in the trunk’s migration files. (We created it in the patch > branch and we know the migration name from that branch, but we never added > the patch migration to the mainline trunk.) The current documentation[1] > describes keeping both the original and the replacement in place until all > databases have migrated past the replacement step (and then deleting the > original and removing the “replaces” attribute from the replacement). The > documentation implies, but does not explicitly state, that the original > step should exist in the list. Our testing shows that the original need > not exist (and we like it this way!). > > * If we go ahead and add a
Migrations: A bug or a feature needing documentation?
[ TL;DR: A migration may use a “replaces” list pointing to migrations that don’t actually exist. This undocumented technique cleanly solves a recurring difficult migration problem. We seek consensus on whether this should become a documented feature or that it is an unexpected side effect subject to change in the future. ] We have found an undocumented behavior in the migration system that gracefully solves the troublesome problem of merging migrations created in parallel development branches. If this behavior should survive, we’ll enter a documentation ticket – but if it’s considered a bug, we’ll need to stay away from it and fall back to the more difficult manual editing approaches we’ve used in the past. The Use Case -- We’re rapidly developing a large multi-tenant application (hundreds of ORM models, thousands of migrations and hundreds of thousands of lines of code so far, with quite a bit of work remaining) punctuated by periodic production releases. We create a source code branch from our mainline development trunk for each production release, just in case we must rapidly issue patches to those production releases. On rare occasions, we’ve had to make a schema change (such as adding a new field) as a patch to a production release, and make a parallel schema change in the mainline development trunk. Of course, this normally causes a migration failure when migrating a production tenant from the patch release up to a later version of the mainline release – since the mainline release has a subsequent migration that adds the same field. We’ve solved this in the past by manually rearranging the dependency order of the mainline trunk migrations (moving the replacement step before other new migrations for this later release), and fiddling with the contents of the django_migrations table to make it look like that mainline step has already been run before running the migrations. We’re unhappy with that approach – it’s both time consuming and error prone. This problem is similar to, but not identical to, that of squashing migrations. (And yes, we do periodically squash our migrations. We have about 600 migration steps at the moment, left over from more than 2,000 originally created. We’ve got another round of squashing coming up soon that should take us to less than 100 migrations – but we have more than a dozen developers adding more migrations every week.) The Discovery --- Through trial and error, we found that our mainline migration step may declare itself as a replacement for the patch step (using the “replaces” attribute) – even if the patch migration itself doesn’t exist in the list of mainline migrations. And if we do this, the migration engine simply works as hoped and our problem vanishes. It’s absolutely wonderful; simple to implement and effective. We love it. New tenants run only the replacement step; tenants migrating from the patch release to the trunk release merely record the replacement step as having been completed without actually executing it; development tenants that never saw the original patch step simply record both the patch step and the replacement as having been completed. It’s great. The Worry -- This approach seems undocumented in three different ways: * The replacement migration is pointing at an original migration that doesn’t exist in the trunk’s migration files. (We created it in the patch branch and we know the migration name from that branch, but we never added the patch migration to the mainline trunk.) The current documentation[1] describes keeping both the original and the replacement in place until all databases have migrated past the replacement step (and then deleting the original and removing the “replaces” attribute from the replacement). The documentation implies, but does not explicitly state, that the original step should exist in the list. Our testing shows that the original need not exist (and we like it this way!). * If we go ahead and add a copy of the patch release’s migration step to the mainline trunk, we introduce a “multiple leaf nodes” graph, since none of the mainline migrations depend upon this “side patch”. However, apparently because there is a declared replacement for this patch step, the migration engine doesn’t raise the “multiple leaf nodes” exception. This seems to be an oversight unless the replacement step is somehow acting as a merge (as if it had a dependency on the patch step) … but we like the way it’s working now, if it were to become necessary to include the original step in the mainline migration list. * We have found that we can have multiple replacement steps all claiming to replace the same original step number. (This conveniently handled a case where multiple migrations were originally created in the trunk, then backported as a single migration into a patch to an earlier production release.) But this
