Database migration failing from umbraco 7.15 to 8.18.3

simonech commented 2 years ago

Which exact Umbraco version are you using? For example: 9.0.1 - don't just write v9

8.18.3

Bug summary

When migrating from 7.15.1 to 8.18.3 the upgrade fails when it tried to re-create all indexes and keys. The list of indexes to process also includes the table ContentVersionCleanupPolicyDto which doesn't exist yet since it's a feature (and table) added in 8.18.

the error we get is

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> Umbraco.Web.Install.InstallException: The database failed to upgrade. ERROR: The database configuration failed with the following message: Cannot find the object \"umbracoContentVersionCleanupPolicy\" because it does not exist or you do not have permissions

Specifics

No response

Steps to reproduce

Migrate from 7.15.1 to 8.18.3

Expected result / actual result

No response

p-m-j commented 2 years ago

What a pain, presumably the workaround is add an intermediate v8 upgrade step e.g. 7.15.1 -> 8.0.0 > 8.18.3

simonech commented 2 years ago

Yeah, annoying workaround

AndyButland commented 2 years ago

Just sharing here also what I mentioned to Umbraco support when they reached out about this... this isn't a very elegant suggestion, but if it's just needed as a one-off to get over this bump, you could:

Install a new instance of 8.18.3
Via SQL Management Studio, generate a script for creating the umbracoContentVersionCleanupPolicy table and then run this on the database you are upgrading.
With that in place, when the part runs that looks to create the indexes, it'll find the table OK and carry on.
When it then hits the part that creates the table, it'll silently skip over the creation as it already exists.

simonech commented 2 years ago

Thank you. Yeah we had tried already that, but one of the migration steps tries to do something with the one of the indexes referenced by the new FK, so it’s a bit messy adding just table w/o FKs and then have it added later. We are migrating 2-3 more sites in the coming months, and I fear that if this issue is not fixed with a more structured approach, with probably many new feautres coming up, it will require a lot of these manual steps to complete a migration.

-- Simone Chiaretta Microsoft MVP ASP.NET - ASPInsider Blog: http://codeclimber.net.nz RSS: http://feeds2.feedburner.com/codeclimber twitter: @simonech

On 5 May 2022 at 08:58:51, Andy Butland @.***) wrote:

Just sharing here also what I mentioned to Umbraco support when they reached out about this... this isn't a very elegant suggestion, but if it's just needed as a one-off to get over this bump, you could:

Install a new instance of 8.18.3
Via SQL Management Studio, generate a script for creating the umbracoContentVersionCleanupPolicy table and then run this on the database you are upgrading.
With that in place, when the part runs that looks to create the indexes, it'll find the table OK and carry on.
When it then hits the part that creates the table, it'll silently skip over https://github.com/umbraco/Umbraco-CMS/blob/8a73bbb900075598b78c5f9edfaab2dbd4e53673/src/Umbraco.Core/Migrations/Upgrade/V_8_18_0/AddContentVersionCleanupFeature.cs#L17 the creation as it already exists.

— Reply to this email directly, view it on GitHub https://github.com/umbraco/Umbraco-CMS/issues/12351#issuecomment-1118232786, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPA5IYHATFVBIKLFCDX6LVINWSVANCNFSM5VBZZVNA . You are receiving this because you authored the thread.Message ID: @.***>

p-m-j commented 2 years ago

it might be enough to update CreateKeysAndIndexes to explictly re-create only for tables altered by DeleteKeysAndIndexes.cs but really the migration should be absolutely explicit about which indexes and keys are to be created.

e.g. this may be enough

public class CreateKeysAndIndexes : MigrationBase
{
    ....

    protected override void Migrate()
    {
        // remove those that may already have keys
        Delete.KeysAndIndexes(Cms.Core.Constants.DatabaseSchema.Tables.KeyValue).Do();
        Delete.KeysAndIndexes(Cms.Core.Constants.DatabaseSchema.Tables.PropertyData).Do();

        // re-create *all* keys and indexes for tables explicitly

        Create.KeysAndIndexes<ContentDto>();
        Create.KeysAndIndexes<ContentTypeDto>();
        ...
        ...
        ...
    }
}

simonech commented 2 years ago

Or maybe in the CreateKeysAndIndexes, check if the table exists before creating the indexes related to the table

-- Simone Chiaretta Microsoft MVP ASP.NET - ASPInsider Blog: http://codeclimber.net.nz RSS: http://feeds2.feedburner.com/codeclimber twitter: @simonech

On 5 May 2022 at 12:38:27, Paul Johnson @.***) wrote:

it might be enough to update CreateKeysAndIndexes to explictly re-create only for tables altered by DeleteKeysAndIndexes.cs https://github.com/umbraco/Umbraco-CMS/blob/release-8.18.3/src/Umbraco.Core/Migrations/Upgrade/Common/DeleteKeysAndIndexes.cs but really the migration should be absolutely explicit about which indexes and keys are to be created.

e.g. this may be enough

public class CreateKeysAndIndexes : MigrationBase { ....

protected override void Migrate()
{
    // remove those that may already have keys
    Delete.KeysAndIndexes(Cms.Core.Constants.DatabaseSchema.Tables.KeyValue).Do();
    Delete.KeysAndIndexes(Cms.Core.Constants.DatabaseSchema.Tables.PropertyData).Do();

    // re-create *all* keys and indexes explicitly

    Create.KeysAndIndexes<ContentDto>();
    Create.KeysAndIndexes<ContentTypeDto>();
    ...
    ...
    ...
}

}

— Reply to this email directly, view it on GitHub https://github.com/umbraco/Umbraco-CMS/issues/12351#issuecomment-1118406766, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPA5KGNX4XJMANSEMFYQLVIOQKFANCNFSM5VBZZVNA . You are receiving this because you were mentioned.Message ID: @.***>

netaddicts commented 2 years ago

As @simonech already suggested, the fix is pretty simple, check if a table exist before trying to delete or create indexes, it should be as simple as that

umbrabot commented 2 years ago

Hi @simonech,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

simonech commented 2 years ago

@p-m-j indeed another options would be use the same list that is in the DeleteKeysAndIndexes because the create key is also executed during the initial migration from v7, so should only create indexes for v7 tables. What do you think?

p-m-j commented 2 years ago

@simonech Both approaches are basically the same, and neither is ideal as the entity classes could still define a key or index which is not possible until a later migration has run but hopefully that's not the case.

My understanding is that these older migrations are to be nuked for v10 final as we expect folks to migrate only from the previous major so if anything can ease the pain from v7 to v8.18 that's fine and this migration shouldn't cause pain regardless moving forwards.

When the build for #12377 is complete if i stuck a pre release on myget would you be able to test?

simonech commented 2 years ago

@p-m-j I can try, but logging off for the week now. Can try next week

p-m-j commented 2 years ago

@simonech - can be found here https://www.myget.org/feed/umbracoprereleases/package/nuget/UmbracoCms.Core/8.18.3-alpha.15245

hfloyd commented 2 years ago

I have been busy over the last couple years migrating v7 sites to vLatest, and foresee that continuing for some time to come. I don't mind having to go through all the major versions (7-latest, 8-latest, 9-latest...), but ideally I wouldn't have to ALSO go through various interim versions to manage these annoying DB issues.

I came across this one today while going from 7.15.7 to 8.18.3. (Fresh v8.18.3 site pointing to SQL DB for v7 site)

I used NuGet to install @p-m-j 's provided update, rebuilt, and ran the upgrade wizard again - and SUCCESS! Thank you so much, @p-m-j !!!

p-m-j commented 2 years ago

Cheers @hfloyd that's good to know, it's worth stating that if you are moving onto v9 there's no need to install the latest v8 minor first, you could just as easily upgrade to 9.5 from 8.0 as 8.18

hfloyd commented 2 years ago

@p-m-j That's a fair point. I have a few "cleanup tools" I like to use during migrations, thus working from the latest is my personal preference. (Additionally it's been my experience that sometimes the migrations from one minor to another don't always go as planned...)

c9mb commented 2 years ago

@p-m-j Just so that's clear in my mind, the intention is to be able to migrate from 7.x to 8.0 to 9.0 to 10.x ? That would sound fine to me. I have quite a number of v7 sites on Cloud and clients who don't want me to change anything - it ain't broke so don't fix it. However, eventually the day will come, and I need to be ready for it.

p-m-j commented 2 years ago

@c9mb yep that’s the theory.

simonech commented 2 years ago

Thx @p-m-j for the fix. I trust @hfloyd judgement since she got into the same issue with her migration. Glad to see it works!

nul800sebastiaan commented 2 years ago

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/12377

umbraco / Umbraco-CMS