Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running Grate on RoundhousE database - all scripts are new #550

Open
Edgaras91 opened this issue Jun 26, 2024 · 13 comments · May be fixed by #551
Open

Running Grate on RoundhousE database - all scripts are new #550

Edgaras91 opened this issue Jun 26, 2024 · 13 comments · May be fixed by #551

Comments

@Edgaras91
Copy link

Edgaras91 commented Jun 26, 2024

Describe the bug
Using grate.sqlserver, HasRun() check is fetching scripts from RoundhousE.GrateScriptsRun which is wrong when trying to run my scripts (xxxx.NewAddressColumns.sql). My scripts already ran and are in RoundhousE.ScriptsRun table.

To Reproduce
Have an existing RoundhousE tables such as "RoundhousE.ScriptsRun". Specify the "RoundhousE" schema in start-up.

Expected behavior
The existing RoundhousE script to not run again.

Desktop (please complete the following information):

  • OS: Windows 11
  • Version: 1.7.4

Context
This in AnsiSqlDatabase class, this is the cache:
private async Task<IDictionary<string, string>> GetScriptsRunCache() => _scriptsRunCache ??= await GetAllScriptsRun();
_scriptsRunCache is shared across all migration scripts (Grate scripts and user scripts) which is wrong. user scripts should either "refresh" the cache using the correct table "ScriptsRun" or separate instance of the AnsiSqlDatabase class, or another property to store this (or something else)

@Edgaras91
Copy link
Author

After checking the codebase more, I think the easiest fix for this, is to make the AnsiSqlDatabase._scriptsRunCache more complex than IDictionary<string, string>? to cater for multiple tables (Schema.GrateScriptsRun and Schema.ScriptsRun since that's what we have now (we didn't have 2 sets of tables with roundhousE).

should be in this kind of format "Schema.GrateScriptsRun", IDictionary<string, string>? and then check ScriptsRunTable property if we have a key for that.

So Dictionary<string, Dictionary<string, string>>

This should allow to cache multiple tables.

@Edgaras91 Edgaras91 linked a pull request Jun 26, 2024 that will close this issue
@Edgaras91
Copy link
Author

@erikbra I wonder if this could get your attention? It's blocking our migration from RoundhousE to Grate.
I have not physically tested the pull request, but I would appreciate it if someone could try to replicate and test it as a reviewer.

@jswright91
Copy link

jswright91 commented Jul 9, 2024

@Edgaras91 was there a problem using the baseline flag when running grate?
If I'm understanding correctly, when running grate on a db that has previously run roundhouse, you do not want grate to re-run all of the scripts that have already been run.

There is a configuration option for grate that handles this logic.

–baseline

  • This instructs grate to mark the scripts as run, but not to actually run anything against the database. Use this option if you already have scripts that have been run through other means (and BEFORE you start the new ones).

I've transitioned about 30 db's from RoundHouse, to Grate with the following process, and have not once run into a single issue:

  1. Backup db, just in case.
  2. Run RH to make sure all of the scripts in your src folder have been applied (or at least logged to the ScriptsRun table)
  3. Run grate with the -baseline flag. (Make sure you use the same schema you were using in RH. You want to make sure you're utilizing all the same helper tables, so you don't have a confusing set of old RH logging tables on the db.)
  4. You're free to create new scripts, and run grate the same as you would have run RH.

@Edgaras91
Copy link
Author

Edgaras91 commented Jul 10, 2024

@Edgaras91 was there a problem using the baseline flag when running grate? If I'm understanding correctly, when running grate on a db that has previously run roundhouse, you do not want grate to re-run all of the scripts that have already been run.

There is a configuration option for grate that handles this logic.

–baseline

  • This instructs grate to mark the scripts as run, but not to actually run anything against the database. Use this option if you already have scripts that have been run through other means (and BEFORE you start the new ones).

I've transitioned about 30 db's from RoundHouse, to Grate with the following process, and have not once run into a single issue:

1. Backup db, just in case.

2. Run RH to make sure all of the scripts in your src folder have been applied (or at least logged to the ScriptsRun table)

3. Run grate with the -baseline flag. (Make sure you use the same schema you were using in RH. You want to make sure you're utilizing all the same helper tables, so you don't have a confusing set of old RH logging tables on the db.)

4. You're free to create new scripts, and run grate the same as you would have run RH.

With "baseline" = true in GrateConfiguration.cs it's not running any scripts, logs in console:

info: Grate.Migration[0] Looking for Update scripts in "D:\Projects\database\app\bin\Debug\net8.0/db/Application\up". These should be one time only scripts.
info: Grate.Migration[0] --------------------------------------------------------------------------------
info: Grate.Migration[0]  No sql run, either an empty folder, or all files run against destination previously.
info: Grate.Migration[0] --------------------------------------------------------------------------------

when I paste the above path in File Explorer, I do see all the files being physically there (build action: Content)

Then when I set baseline back to false, it tries to run 1st script (which it shouldn't).

Error:

grate.Exceptions.MigrationFailed: 'Migration failed due to errors'
OneTimeScriptChanged: 201801241351.somescript.sql: Script has changed since the last time it was run. By default this is not allowed - scripts that run once should never change. To change this behavior to a warning, please set WarnOnOneTimeScriptChanges to true and run again. Stopping execution.

@jswright91
Copy link

@Edgaras91

Grate should be picking up files in the directories that it's configured to look at.
It LOOKS like you are dumping SQL files in a compiled directory from your .net app. Are we sure those SQL files exist at the time grate runs? Or is grate running BEFORE files get dumped into that directory?

Would you mind sharing what configuration options you are using, and how you're calling grate?

@Edgaras91
Copy link
Author

Edgaras91 commented Jul 11, 2024

They do exist when compiled, before running the application. They do get picked up when baseline = false, so I assume with baseline = true they are simply ignored or whatever, but not run

        var accountsMigrationConfig = new GrateConfiguration
        {
            Baseline = false,
            CommandTimeout = 1800,
            RepositoryPath = configuration.GetValue<string?>("RepositoryPath"),
            Version = typeof(Program).Assembly.GetName().Version!.ToString(3),
            SchemaName = "RoundhousE",
            NonInteractive = true,
            CreateDatabase = false,
            SqlFilesDirectory = $"{Path.GetDirectoryName(Assembly.GetEntryAssembly()!.Location)}/db/Accounts}",
            ConnectionString = configuration.GetConnectionStringsWithBackwardsCompatibility(ConfigurationExtensions.ConfigDatabase.Accounts)
        };

@Edgaras91
Copy link
Author

Can anyone help me understand if I am doing something wrong? I didn't think it would be this difficult to use this project on existing RoundhousE setup

@erikbra
Copy link
Owner

erikbra commented Jul 24, 2024

@Edgaras91 - I'm not 100% sure that you are seeing the correct wrong behaviour here. I worked a bit on the cache yesterday, and it seems. to be working as expected.

The tables GrateScriptsRun, GrateScriptsRunErrors and GrateVersion are tables for versioning the grate structure itself. Every time you run a migration, there will first be two internal migrations run, which run against these tables, to see if there are any changes to the internal grate tables that need to be run (change in the structure of these tables, e.g.) There was an error, #524 , where one of the internal migrations was always run in baseline mode, which doesn't run any of the scripts, but register them as run. This one is fixed now, and will be part of 1.8.0 (to come). However, all of this is only concerning the grate tables themselves, and should not affect the running of the user scripts.

I see @jswright91 writes that he has used baseline mode the first time running grate when migrating from RoundhousE. The intent is that this should not be necessary, if specifying RoundhousE as the grate schema to use. But of course, there might be bugs.

I don't own a Windows machine, and I have no legacy RoundhousE controlled database available to be able to test this properly. Would any of you be able to provide a minimal sample of a RoundhousE database, especially the values in the RoundhousE tables (hash, etc), as a backup file or something, where you see these problems, so that I can look into them? Because, as i am aware, this should "just work" out of the box, but it's been quite a while since I've tested it.

@Edgaras91
Copy link
Author

I just tried it again with version 1.8.0, running on the existing old roundhousE database, with RoundhousE tables pre-populated, with baseline false, and it just keeps trying to execute my up (one-time) scripts even though they ran before. I did not try to debug grate source code at this time.

It is flagging that the scripts have changed. I set the WarnOnOneTimeScriptChanges to true, and it is trying to execute the scripts.

I used the same configs as shared earlier.

Unfortunately, we can't migrate to grate because of this and I hope someone will try the latest version of grate on an ex roundhouse database and replicate my issue.

@hoangthanh28
Copy link
Contributor

This can be related to this discussion #579. I will take a look later this week.

@erikbra
Copy link
Owner

erikbra commented Sep 21, 2024

For info, @Edgaras91 - the RoundhousE.GrateScriptsRun (if you specify RoundhousE as the grate schema) is used internally by grate. grate still uses ScriptsRun, ScriptsRunErrors and Version tables, in the schema you specified, to handle versioning of your scripts. I have created an issue, #583 , to improve this documentation.

But, if you do not try to force Grate to use any special names for ScriptsRun, ScriptsRunErrors, and Version tables, does it still try to run all scripts again, if you have an existing RoundhousE database? The intention is that this should "just work", but there might be some errors in the hash algorithm or other. Unfortunately, I don't have a Windows machine, so I don't have any possibility to create a RoundhousE database to test on. Do you have the option to create a database, or alternatively share an example of one row in your existing RoundhousE.ScriptsRun database (both script text and hash), so that I can make sure that we create the hash exactly the same way as RoundhousE did?

@robbirdht
Copy link

Hi there,

First of all, a massive thanks to everyone working on this project, it's great to see this giving a cross platform future to our roundhouse projects!

I am also trying to migrate a RoundhousE managed database and bumping into an issue where Grate is running all the previous scripts again when I was expecting them to be skipped.

Looking into the [ScriptsRun] table, I can see that the script content and hash match, but the script_name doesn't, which I am assuming is the reason that it is being run.
Interestingly my scripts are organised into sub-folders within the up folder, based on incremental sprint numbers. It is the subfolder part which is different. Roundhouse was ignoring the folder and just storing the script file name alone in the script_name field, whereas Grate is storing the folder name in the script_name field too.

I have tried testing this by manually altering the ScriptsRun table to add the folder name to a row and then I can see that it gets skipped as expected.

I have hundreds of scripts in around 60 folders, so it would be grate :) if there was a way to control this matching behaviour.
image

@robbirdht
Copy link

Ok posted a little too soon, I couldn't find anything that looked promising in the configuration options page, however after digging into the code to try to understand the matching I came across an undocumented flag which seems to solve my problem!

Using --ignoredirectorynames when running grate is now matching my previous RoundhousE database perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants