Summary: | Store each database migrations state in database | ||
---|---|---|---|
Product: | Koha | Reporter: | Julian Maurice <julian.maurice> |
Component: | Architecture, internals, and plumbing | Assignee: | Julian Maurice <julian.maurice> |
Status: | In Discussion --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | dcook, jonathan.druart, martin.renvoize, olli-antti.kivilahti, paul.poulain, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34088 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 25078 | ||
Bug Blocks: | |||
Attachments: |
Bug 27880: Database migrations in separate files
Bug 27880: Store each database migrations state in database Bug 27880: Store each database migrations state in database Bug 27880: Minor optimizations for Koha::Migrations::pending_migrations |
Description
Julian Maurice
2021-03-05 12:01:51 UTC
Created attachment 117835 [details] [review] Bug 27880: Database migrations in separate files The goals of this patch are: - stop the growth of updatedatabase.pl file (which is nearly 24k lines long at the moment) - reduce the manual work for RM/RMaints (copy/paste of code from atomicupdate to updatedatabase.pl + increase of Koha version) - provide a simple framework for a better control of the database update process Database migrations are regular Perl scripts that return migration metadata and code in a hashref. Their filename is meaningful. It should always start with a timestamp (in the form YYYYMMDDHHMMSS), which is used for executing migrations in the correct order. It should also contain a short description. Once a migration has been successfully executed, its name is stored into a new table `executed_migrations`. This is how we avoid executing a migration twice. With this new system, the 'Version' system preference becomes useless, as well as the 4th part of the Koha version. A good part of the patch is about adapting the code to these changes. Test plan: 1. Apply patch and restart starman 2. Go to the staff interface, you should be redirected to the installer 3. Enter your credentials, click on "Continue to the next step" until you arrive at the "Update database" step 4. You should now see the following message: Pending migrations: 20201002135449-create-executed-migrations-table.pl Click on "Update your database" 5. Now you should see Update report : Migration done: 20201002135449-create-executed-migrations-table.pl (Create table executed_migrations) Everything went okay. Update done. 6. Log into Koha 7. Verify that the About page shows the correct version 8. Verify that the 'Help' link in the top right corner redirects to the correct version of the manual 9. Now, do a fresh install and verify that the executed_migrations table exists and is not empty at the end of the install process *** Bug 7167 has been marked as a duplicate of this bug. *** So.. bits of this ended up getting merged into bug 25078.. I'm not sure what's still to do.. I think the recording of which updates have run which this bug does is a good idea still.. allowing us to make update idempotent by checking for previous runs of the update rather than trying to be clever with that actual statements themselves. Thoughts Julian.. is this still something you're happy to pursue? Yes, this is something I still want to work on. I'll try to rebase the patch soon so it can be tested Created attachment 127998 [details] [review] Bug 27880: Store each database migrations state in database This patch is the continuation of bug 25078. It improves it by decorrelating database migrations from Koha version number, which allows to prevent that a database migration is executed twice, even if present in two different stable releases. Database migrations are regular Perl scripts that return migration metadata and code in a hashref. Their filename is meaningful. It should always start with a timestamp (in the form YYYYMMDDHHMMSS), which is used for executing migrations in the correct order. It should also contain a short description. Once a migration has been successfully executed, its name is stored into a new table `executed_migrations`. This is how we avoid executing a migration twice. With this new system, the 'Version' system preference becomes useless, as well as the 4th part of the Koha version. A good part of the patch is about adapting the code to these changes. Test plan: 1. Apply patch and restart starman 2. Go to the staff interface, you should be redirected to the installer 3. Enter your credentials, click on "Continue to the next step" until you arrive at the "Update database" step 4. You should now see the following message: Pending migrations: 20201002135449-create-executed-migrations-table.pl Click on "Update your database" 5. Now you should see Update report : Migration done: 20201002135449-create-executed-migrations-table.pl (Create table executed_migrations) Everything went okay. Update done. 6. Log into Koha 7. Verify that the About page shows the correct version 8. Verify that the 'Help' link in the top right corner redirects to the correct version of the manual 9. Now, do a fresh install and verify that the executed_migrations table exists and is not empty at the end of the install process The patch is rebased and works, but there is still some work to do: - figure what to do with the already existing db_revs/<version>.pl (I believe we can leave it like that, but maybe there's another, cleaner solution) - pass the same parameters to the 'up' subroutine as with the db_revs system (for now only $dbh is passed, not the $out parameter) *** Bug 14698 has been marked as a duplicate of this bug. *** (In reply to Julian Maurice from comment #0) > The goals of this patch are: > - stop the growth of updatedatabase.pl file (which is nearly 24k lines long > at the moment) > - reduce the manual work for RM/RMaints (copy/paste of code from > atomicupdate to updatedatabase.pl + increase of Koha version) > - provide a simple framework for a better control of the database update > process It feels like the first 2 goals have been achieved with the move to atomicupdate files, leaving only the part for saving run updates to the database. I am not sure about the benefit of that as we already can tell what has been run from the version - but I might be persuaded? The goal is to reduce manual work. RM/RMaint won't need to rename the file and update the version number. There will be no distinction between a "dev update" (located in atomicupdate directory) and a "prod update" (located in db_revs directory). It becomes just like any other piece of code: it doesn't need to be moved once pushed/backported. Also DB updates will never be executed twice (currently this can happen when switching from a stable version to another, since DB updates are often backported) so they won't need to be idempotent and should be easier to write. For instance there is no need to check if a column exists before adding it because that column cannot exist before the update was executed. I'm a fan of storing database migrations state in the database. I use that strategy with other apps I write/support. But I'm not sure I follow the position below... (In reply to Julian Maurice from comment #9) > RM/RMaint won't need to rename the file and update the version number. There > will be no distinction between a "dev update" (located in atomicupdate > directory) and a "prod update" (located in db_revs directory). It becomes > just like any other piece of code: it doesn't need to be moved once > pushed/backported. Depending on when the update is pushed/backported, wouldn't the RM/RMaint need to rename the file so that it fits into the right order? If the developer creates a migration in 2021 but it isn't pushed until 2023, it'll be out of order with all the other migrations. I suppose you could argue that it'll be applied at the right time because all the other migrations would've been applied already, but I don't think that would work for upgrades across multiple versions. But I might be misunderstanding what you're proposing here. > Also DB updates will never be executed twice (currently this can happen when > switching from a stable version to another, since DB updates are often > backported) so they won't need to be idempotent and should be easier to > write. > For instance there is no need to check if a column exists before adding it > because that column cannot exist before the update was executed. I reckon it's always good practice to check before adding things. I've noticed a lot of database discrepancies, so you never know what Frankenstein's monster of a database you might have. (In reply to Julian Maurice from comment #0) > The goals of this patch are: > - stop the growth of updatedatabase.pl file (which is nearly 24k lines long > at the moment) > - reduce the manual work for RM/RMaints (copy/paste of code from > atomicupdate to updatedatabase.pl + increase of Koha version) > - provide a simple framework for a better control of the database update > process Other work has already stopped the growth of updatedatabase.pl, fortunately. I don't understand the manual work for RM/RMaints so maybe we can get a more in-depth explanation for that? I do want to reiterate that I think recording database migrations in the database is a good idea. Although what do we do for historical migrations? Do we just choose an arbitrary point to start recording from? Julian, you might also find bug 34088 interesting. In that case, I'm just short circuiting when the database version is equal to the code version, but if we stored migrations in the database I was wondering if there could be an easy way of telling whether or not there were any outstanding migrations to run (in a high performance way). I think Koha::Migrations->pending_migrations is actually a significant win for these patches. It gets a big hashref of all past migrations, and then it checks in O(1) time whether a file has already been used. So we only have 1 database hit, and we have an efficient check per file. Of course, the time it takes to check for pending migrations will degrade over time as the list of migration files grows. But that's a problem that we already have. So we'd still have a net win by moving to this mechanism. (In reply to David Cook from comment #10) > Depending on when the update is pushed/backported, wouldn't the RM/RMaint > need to rename the file so that it fits into the right order? If the > developer creates a migration in 2021 but it isn't pushed until 2023, it'll > be out of order with all the other migrations. I think it's not a problem. Migrations don't need to be run in "push order". What matters is that a migration is executed after any other migrations it might rely on, which are all the migrations already existing in master at the time of writing the new migration (+ any migrations contained in bug reports dependencies). The timestamp prefix ensures that. > I suppose you could argue that it'll be applied at the right time because > all the other migrations would've been applied already, but I don't think > that would work for upgrades across multiple versions. I don't understand, or I don't agree :) If stable has migrations A B C D E and oldstable has only A C E, when switching for oldstable to stable, migrations B and D will be executed. Why do you think it wouldn't work ? > I reckon it's always good practice to check before adding things. I've > noticed a lot of database discrepancies, so you never know what > Frankenstein's monster of a database you might have. Yes I might have been over optimistic with this. Things can go wrong even when migrations are executed only once. (In reply to David Cook from comment #11) > I don't understand the manual work for RM/RMaints so maybe we can get a more > in-depth explanation for that? Move/rename of the atomicupdate file + increase of Koha version. It won't be necessary with these patches. > I do want to reiterate that I think recording database migrations in the > database is a good idea. Although what do we do for historical migrations? > Do we just choose an arbitrary point to start recording from? Yes. The arbitrary point would be when this patch is pushed, which will record the first migration. It might be a good idea to push it early in a release cycle, before any migration. Since it removes the 4th part of the version number, it may cause confusion if we do it after other migrations have been included (23.11.06.000 -> 23.11.06.001 -> 23.11.06 (???)) (In reply to David Cook from comment #12) > Julian, you might also find bug 34088 interesting. > > In that case, I'm just short circuiting when the database version is equal > to the code version, but if we stored migrations in the database I was > wondering if there could be an easy way of telling whether or not there were > any outstanding migrations to run (in a high performance way). Hmm.. Probably not :/ (In reply to David Cook from comment #13) > I think Koha::Migrations->pending_migrations is actually a significant win > for these patches. > > It gets a big hashref of all past migrations, and then it checks in O(1) > time whether a file has already been used. > > So we only have 1 database hit, and we have an efficient check per file. > > Of course, the time it takes to check for pending migrations will degrade > over time as the list of migration files grows. But that's a problem that we > already have. It would be interesting to see how much time it takes to process a directory of 100, 1000 and 10000 entries (we are getting close to 1500 updates actually in master) I don't have time to reply right now but hopefully I will tomorrow. (Lots of holidays at the moment so might be a bit out of touch.) Hm, you pretend like sequence of execution does not matter, but indeed it does. A FK can only be added to a table that already exists, a column can only be added AFTER another column that is already there etc. The sequence and the way we sometimes need to shift data make it necessary that even if updates are idempotent or only run once, the sequence plays a role. (In reply to Julian Maurice from comment #14) > It would be interesting to see how much time it takes to process a directory > of 100, 1000 and 10000 entries (we are getting close to 1500 updates > actually in master) Just ran a quick test: Time to execute pending_migrations with an empty executed_migrations table and: - 100 migration files: <1ms - 1000 migration files: ~1.2ms - 10000 migration files: ~6ms Time to execute pending_migrations with a full executed_migrations table and: - 1000 migration files: ~1.6ms - 10000 migration files: ~12ms Note that I got these times with two small modifications I made to pending_migrations, which I will submit with the rebased patch, they save a few ms in the "10000 files" test. One way to avoid slowdowns as the number of files grows is to check for pending migrations only once at the start of the application, and not at every request. (In reply to Katrin Fischer from comment #16) > Hm, you pretend like sequence of execution does not matter, but indeed it > does. A FK can only be added to a table that already exists, a column can > only be added AFTER another column that is already there etc. The sequence > and the way we sometimes need to shift data make it necessary that even if > updates are idempotent or only run once, the sequence plays a role. Sequence does matter indeed. But there are several different valid sequences of execution. You can't write a migration to add a FK to a table before the migration to add the table has been written, so the timestamp for the FK migration will always be after the timestamp for the table migration. So these two migrations will always be executed in the correct order. But if you have a migration that adds a column to table A and another migration that adds a column to table B, the order of execution of these two migrations does not matter. Created attachment 153006 [details] [review] Bug 27880: Store each database migrations state in database This patch is the continuation of bug 25078. It improves it by decorrelating database migrations from Koha version number, which allows to prevent that a database migration is executed twice, even if present in two different stable releases. Database migrations are regular Perl scripts that return migration metadata and code in a hashref. Their filename is meaningful. It should always start with a timestamp (in the form YYYYMMDDHHMMSS), which is used for executing migrations in the correct order. It should also contain a short description. Once a migration has been successfully executed, its name is stored into a new table `executed_migrations`. This is how we avoid executing a migration twice. With this new system, the 'Version' system preference becomes useless, as well as the 4th part of the Koha version. A good part of the patch is about adapting the code to these changes. Test plan: 1. Apply patch and restart starman 2. Go to the staff interface, you should be redirected to the installer 3. Enter your credentials, click on "Continue to the next step" until you arrive at the "Update database" step 4. You should now see the following message: Pending migrations: 20201002135449-create-executed-migrations-table.pl Click on "Update your database" 5. Now you should see Update report : Migration done: 20201002135449-create-executed-migrations-table.pl (Create table executed_migrations) Everything went okay. Update done. 6. Log into Koha 7. Verify that the About page shows the correct version 8. Verify that the 'Help' link in the top right corner redirects to the correct version of the manual 9. Now, do a fresh install and verify that the executed_migrations table exists and is not empty at the end of the install process Created attachment 153007 [details] [review] Bug 27880: Minor optimizations for Koha::Migrations::pending_migrations Can save a few milliseconds when there are a lot (> 1000) of migration files Note that comment 6 is still valid (there is still work to be done). How and when is the timestamp set? (In reply to Katrin Fischer from comment #22) > How and when is the timestamp set? The timestamp is in the filename (e.g. 20230704114126-add-syspref.pl if you create the file on 2023-07-04 at 11:41:26) (In reply to Julian Maurice from comment #23) > (In reply to Katrin Fischer from comment #22) > > How and when is the timestamp set? > > The timestamp is in the filename (e.g. 20230704114126-add-syspref.pl if you > create the file on 2023-07-04 at 11:41:26) But I don't know when it will be pushed, to renaming will be necessary for the RM like they do now. (In reply to Julian Maurice from comment #17) > (In reply to Julian Maurice from comment #14) > > It would be interesting to see how much time it takes to process a directory > > of 100, 1000 and 10000 entries (we are getting close to 1500 updates > > actually in master) > > Just ran a quick test: > > Time to execute pending_migrations with an empty executed_migrations table > and: > - 100 migration files: <1ms > - 1000 migration files: ~1.2ms > - 10000 migration files: ~6ms > > Time to execute pending_migrations with a full executed_migrations table and: > - 1000 migration files: ~1.6ms > - 10000 migration files: ~12ms > > Note that I got these times with two small modifications I made to > pending_migrations, which I will submit with the rebased patch, they save a > few ms in the "10000 files" test. > > One way to avoid slowdowns as the number of files grows is to check for > pending migrations only once at the start of the application, and not at > every request. I'm a bit surprised that pending_migrations ran that fast. It takes me a lot longer just to load C4::Context and get a database handle via C4::Context->dbh on koha-testing-docker. Did you run that on koha-testing-docker or on a server? -- Regarding only checking for migrations at the start of the application, that's a good point. I've noticed that's what other (persistent) applications do as well. I think we have consensus at this point that Koha should always be run persistently, so I think that is a reasonable move to make. That said, I'm less worried about slowdowns with the web UI vs updatedatabase.pl, since updatedatabase.pl is where I notice the worst performance impacts when installing a new koha-common package. What do you think about bug 34088 in this context? If the code version and the database version match and there are no atomic updates to run, it exits out of updatedatabase.pl. Koha::Migrations does look pretty minimal, but I found C4::Context to be too slow, so I used Koha::Database::dbh() and Koha::Config for a database handle and config since they were much faster to load and use. (In reply to Julian Maurice from comment #18) > Sequence does matter indeed. But there are several different valid sequences > of execution. > You can't write a migration to add a FK to a table before the migration to > add the table has been written, so the timestamp for the FK migration will > always be after the timestamp for the table migration. So these two > migrations will always be executed in the correct order. > But if you have a migration that adds a column to table A and another > migration that adds a column to table B, the order of execution of these two > migrations does not matter. I understand your logic here, but I just wonder if you're missing some scenarios. I'm struggling to think of them, but there's just something bothering me in the back of my mind. At the moment, I wonder about backporting causing problems... Scenario: Version 23.05 and Master - Bug 1 : Add column A to Table A Master: - Bug 2 : Modify Column A in Table A - Bug 3 : Drop Column A in Table A 23.05: - Bug 3 : Drop Column A in Table A (backported) -- When you upgrade from 23.05 to 23.11, the migration for Bug 2 will exist on the file system, but it won't be marked as executed in the database, so Koha will try to apply the migration for Bug 2, and it should generate an error. (But to be fair, I think that this might actually be a problem with the current db_revs system as well... I'd have to test it...) (In reply to David Cook from comment #26) > (But to be fair, I think that this might actually be a problem with the > current db_revs system as well... I'd have to test it...) Actually, it might not be a problem with the current version, because db_revs also check their version against the database version. (In reply to David Cook from comment #25) > I'm a bit surprised that pending_migrations ran that fast. It takes me a lot > longer just to load C4::Context and get a database handle via > C4::Context->dbh on koha-testing-docker. Did you run that on > koha-testing-docker or on a server? I opened the database connection before starting the timer. I wanted to measure only the database query + listing the files. Here's the code: https://pastes.io/mf644kcqog > What do you think about bug 34088 in this context? If the code version and > the database version match and there are no atomic updates to run, it exits > out of updatedatabase.pl. Koha::Migrations does look pretty minimal, but I > found C4::Context to be too slow, so I used Koha::Database::dbh() and > Koha::Config for a database handle and config since they were much faster to > load and use. C4::Context is useless in Koha::Migrations and can be replaced by Koha::Database. But it will still be slower than comparing two version numbers (In reply to David Cook from comment #27) > (In reply to David Cook from comment #26) > > (But to be fair, I think that this might actually be a problem with the > > current db_revs system as well... I'd have to test it...) > > Actually, it might not be a problem with the current version, because > db_revs also check their version against the database version. With db_revs, in your scenario the order of update would be (23.05) Bug 1 -> Bug 3 -> (switch to 23.11) Bug 2 -> Bug 3 So I think we have the same problem. The only difference is that Bug 3 is executed twice. (In reply to Katrin Fischer from comment #24) > But I don't know when it will be pushed, to renaming will be necessary for > the RM like they do now. The timestamp must be set to the time of writing, and it must not be changed at the time of push (In reply to Julian Maurice from comment #29) > (In reply to Katrin Fischer from comment #24) > > But I don't know when it will be pushed, to renaming will be necessary for > > the RM like they do now. > > The timestamp must be set to the time of writing, and it must not be changed > at the time of push Sorry, but I don't believe that makes sense. If the timestamp determines sequence, we can't do that. We have patches waiting years, they would "slip between" previous patches where they might not fit - arguments about sequence of database changes have been made before. I also think there is a point about comparing versions - we cannot slow down Koha that needs to constantly check if the database needs updating. If this makes the check slower, because much more complex, that would be an issue. (In reply to Katrin Fischer from comment #30) > (In reply to Julian Maurice from comment #29) > > The timestamp must be set to the time of writing, and it must not be changed > > at the time of push > > Sorry, but I don't believe that makes sense. If the timestamp determines > sequence, we can't do that. We have patches waiting years, they would "slip > between" previous patches where they might not fit - arguments about > sequence of database changes have been made before. If I understand correctly you are saying that database schema modifications must be applied in a strict order. But currently we have that only if we stay on master. When jumping from stable versions to stable versions, the order is not guaranteed. Let's take an example: If I'm on 22.05.10 and I jump to 22.11.06 and then to 23.05.01 the order of migrations will be (bug numbers): - [jump to 22.11.06] - [not 32330] (already applied because backported on 22.05), - 30642, - ... - [jump to 23.05.01] - [not 32330] (already applied because backported on 22.05) - 27424 - 20256 - [not 30642] (backported on 22.11) - ... To summarize: 30642 -> ... -> 27424 -> 20256 -> ... But if I'm on 22.05.10 and I jump to 23.05.01, the order will be 27424 -> 20256 -> 30642 -> ... 30642 can be applied before or after 27424 depending on how often Koha is updated. Another example: bugs 33004 and 32799 are both in 22.11 and 23.05 but they were pushed in a different order. If on 22.11 the order will be 33004 -> ... -> 32799, but if going directly to 23.05 it will be 32799 -> 33004 I understand that doing it the way I suggest "feels wrong". It's not intuitive. But I encourage you to search and find an example that may cause problems with this system (and that doesn't cause problems with the current one). If someone can find one I'd be happy to close this bug. > I also think there is a point about comparing versions - we cannot slow down > Koha that needs to constantly check if the database needs updating. If this > makes the check slower, because much more complex, that would be an issue. It really is a minor slowdown. 1.5ms for 1000 migration files (which represents two-thirds of the whole Koha history). But I agree it's an unnecessary slowdown, and the check can moved to the start of the application, as suggested in comment 17. Because it's always faster to do nothing :) Generally speaking, I think that I'm in favour of this work, but I suppose we're all risk averse. If for some reason it does end up causing problems and we have to revert, would RM/RMaints just end up converting migrations into db_revs? If they're happy to do that, then it sounds like there might not actually be too much risk? Although one might argue that we might not notice the problem until the migrations are too well entrenched... When in doubt, I like to look at existing implementations and resources. Ruby on Rails is known for its migrations: https://guides.rubyonrails.org/v3.2/migrations.html#what-s-in-a-name "For example Alice adds migrations 20080906120000 and 20080906123000 and Bob adds 20080906124500 and runs it. Alice finishes her changes and checks in her migrations and Bob pulls down the latest changes. When Bob runs rake db:migrate, Rails knows that it has not run Alice’s two migrations so it executes the up method for each migration. Of course this is no substitution for communication within the team. For example, if Alice’s migration removed a table that Bob’s migration assumed to exist, then trouble would certainly strike." This is similar to what I was saying at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27880#c26 I don't know enough about the RM/RMaint process, but it sounds like they'll need to be very careful with backporting any migrations so that they don't allow conflicting migrations / migrations will need to be written carefully. But I don't think that's a reason not to proceed. Looking at backported db_revs and I think it is like Julian says. The following two files are equivalent: ./installer/data/mysql/db_revs/221102001.pl ./installer/data/mysql/db_revs/221200004.pl When you upgrade from 22.11.03 to 23.05.03, you will run ./installer/data/mysql/db_revs/221200004.pl even though ./installer/data/mysql/db_revs/221102001.pl has already run. The database migration system actually should be better because you're less likely to run the same migration script twice. The migration for "./installer/data/mysql/db_revs/221102001.pl" would be pushed to master then backported to 22.11.03. You run it as part of 22.11.03, which means you don't run it again when you're upgrading to 23.05.03. Nice. -- The only issue would be if there's a migration in 23.05.03 which conflicts with the database coming from 22.11.03, but that would already be a problem with updatedatabase.pl and db_revs anyways. -- So overall I think it should be OK in that respect. (In reply to Julian Maurice from comment #31) > It really is a minor slowdown. 1.5ms for 1000 migration files (which > represents two-thirds of the whole Koha history). > But I agree it's an unnecessary slowdown, and the check can moved to the > start of the application, as suggested in comment 17. Because it's always > faster to do nothing :) If we move it to the start of the application, I think we'd have to process upgrades automatically behind the scenes and not via the UI. That would leave the UI just for the initial install I think? Honestly, for most people using Koha packages, that should be the workflow they experience anyway, so I think that should work out well. Would you do this in plack.psgi or somewhere else? (In reply to David Cook from comment #35) > If we move it to the start of the application, I think we'd have to process > upgrades automatically behind the scenes and not via the UI. It's an interesting option. But maybe Koha should warn before making changes to the database schema. Or administrators should be able to disable this feature if they want greater control. Another option is to simply refuse to start and tell the administrator it should run updatedatabase.pl Another one is to start a minimal app that includes only the web installer (but that would probably require another restart after the web installer is done). > Would you do this in plack.psgi or somewhere else? plack.psgi, and Koha::App::{Intranet,Opac}::startup for mojolicious apps (In reply to Julian Maurice from comment #36) > (In reply to David Cook from comment #35) > > If we move it to the start of the application, I think we'd have to process > > upgrades automatically behind the scenes and not via the UI. > > It's an interesting option. But maybe Koha should warn before making changes > to the database schema. Or administrators should be able to disable this > feature if they want greater control. > Another option is to simply refuse to start and tell the administrator it > should run updatedatabase.pl > Another one is to start a minimal app that includes only the web installer > (but that would probably require another restart after the web installer is > done). I suppose another option would be to do the install/upgrade check at startup time and set a variable in a global/singleton like Koha::Installer, which would be checked by each worker... If "need_upgrade" is set, then they repeat the check, because only 1 worker will likely complete the install/upgrade process, and the other workers would be working with stale data... I wonder sometimes how many people use the Debian packages versus other installation methods... The Debian packages automatically run the database upgrade so the upgrade check at startup time should always be false in a test/prod setting. For developers, expecting them to manually run "koha-upgrade-schema" seems fair to me. So I imagine we're considering people who do source/tarball installs, or git/dev installs? In theory, when they're doing installs/upgrades, they're using "make", so maybe we should add a database upgrade step there OR just add a warning that they should manually run it. |