Skip to content

[3.0] Offers to put the database back when an upgrade did not finish (part 2 of 2) - #9657

Open
albertlast wants to merge 22 commits into
SimpleMachines:release-3.0from
albertlast:3.0/migration-rollback-offer
Open

[3.0] Offers to put the database back when an upgrade did not finish (part 2 of 2)#9657
albertlast wants to merge 22 commits into
SimpleMachines:release-3.0from
albertlast:3.0/migration-rollback-offer

Conversation

@albertlast

@albertlast albertlast commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

Part 2 of 2. #9656 writes down what an upgrade found and can put it back; this
part is how an admin reaches that.

This PR contains #9656's commits as well, because its base has to be
release-3.0. Please merge #9656 first — the six commits on top of it are:

The offer

An upgrade that stopped part way leaves a run with no finishing time on it. When
the upgrader finds one, the Upgrade Options step says so and offers a second way
out beside carrying on:

An unfinished upgrade
An upgrade from 2.1.7 was started Today at 04:04 PM and did not finish. You can
carry on with it, or put the database back the way it was before it started.
Only the database is put back: the files on disk are left alone.
[ Put the database back ]

Only a run that actually took a backup is offered, since nothing else has the rows
to put back. A finished upgrade is never offered: putting a working forum back is
something an admin should have to go looking for, not something the upgrader
suggests.

Afterwards

The done notice says what state the forum is now in, rather than leaving the admin
on a page with nowhere to go:

The database has been put back the way it was before the upgrade started. The
files on disk are still SMF 3.0 Alpha 4, so the forum will not run until you
either [start the upgrade again] or put your old files back from a backup.

Following that link starts a fresh run, which takes its own backup — the rolled
back run is not adopted and not overwritten.

grafik grafik grafik
On the command line

php upgrade.php --rollback does the same thing without the browser.

Also here

Three fixes that only a rollback-then-upgrade cycle could expose:

  • A re-upgrade adopted the run it had just rolled back, and so took no backup of
    its own. currentRun() now passes over a run that has been undone.
  • The version to go back to was read from Config::$modSettings in memory, which
    by then named the version that had just been removed. It comes from the run's
    own version_from instead.
  • A rollback left maintenance_tool_progress behind, pointing the next upgrade at
    a step that no longer applied to the database in front of it. In one run that
    skipped the entire v2_1 migration batch while printing "Upgrade complete!".
Verified

The offer, the button, the rollback and the upgrade after it, driven through a
browser on MySQL and on the command line on both engines. .docker/try-rollback-offer.sh
sets up an unfinished run and asks the upgrader to undo it, so the offer can be
exercised without having to time a kill.

Issues References (Fixes|Related|Closes)

  1. Part 2 of 2, on top of [3.0] Records what an upgrade found, so it can be put back (part 1 of 2) #9656

@albertlast
Keeps what a table looked like before the migrations
e04fe3c 
A backup table holds the rows and, on both engines, very little else:
LIKE copies columns and their types but no indexes, no keys and no
sequence, so what an admin has afterwards is not enough to rebuild the
table it came from.

The migrations table is somewhere a migration can leave what it knew
before it acted, grouped by the run that wrote it. The backup step uses
it to keep each table's definition. A run that is interrupted and
started again is the same run, finds what it already wrote and leaves it
alone, so the definition stays the one taken before anything changed.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Copies rows into a backup without copying its source's sequence
cbba1eb 
A default is copied as the expression it is written with, so a column
fed by a sequence left backup_smf_calendar_holidays.id_holiday pointing
at smf_calendar_holidays_seq. The backup became an object that sequence
could not be dropped without, and HolidaysToEvents drops it.

Nothing is lost by leaving the defaults out now: the table's definition
is recorded before the migrations run, and it holds the defaults along
with the indexes, the keys and the sequence that a copy never carried.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Keeps the run's identity where a killed process cannot take it
9e65c10 
The progress data in Settings.php is written by preExit(), which the
command line reaches only once the upgrade has finished -- and a killed
process never reaches at all -- so on that path nothing is ever written
and every attempt would look like a new run.

The settings table is what both ways of running this share and what
survives being killed, so the id lives there, and the step that records
the new version forgets it again.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Gives each upgrade a row of its own
c65554b 
The detail table alone had nowhere to say what a run was, and the id had
to be parked in a setting to survive being killed. A run now has a row:
where it started from, who started it, when it was last heard from, and
where it had got to.

A run is open until something says it finished, so a killed process
leaves its row behind and the next attempt finds it and carries on as
the same run. The position is written as each substep begins, which is
a record of where an upgrade stopped rather than a place to resume
from: restarting still begins at the top.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Leaves a backup this run already took alone
2f94acb 
backup_table() drops the backup table before it writes it, so an upgrade
that is started again after being interrupted replaces a copy of the
database as the admin had it with a copy of it half migrated. The run
now records each table it has copied, and passes over the ones it did.

This is what the copies are for. The definitions taken alongside them
describe the tables as they were, and neither is worth much if the rows
beside them describe something else.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Records the functions a table's definition leans on
8d29787 
An index can be built over an expression rather than a column, and
members has one over indexable_month_day(birthdate). The SQL that
rebuilds the table names the function without saying what it is, so a
definition on its own cannot rebuild the table it describes.

The upgrader's own tables are left out of the backup at the same time.
A copy of the record of what was copied helps nobody putting a forum
back, and restoring it would put back an older account of the run doing
the restoring.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Notes what a setting held before the upgrade changed it
3ddd6ac 
A database put back to the shape it had is not a working forum if
Settings.php still describes the one it was upgraded to. db_character_set
and db_mb4 say what the database is, and after a rollback they would be
saying it about a database that is no longer there.

Only the settings the upgrade writes are noted, and only the first time
each is touched, so this is a note of what to put back rather than a
copy of the file. Settings.php holds the database password, and a copy
of that inside the database would be in every dump taken from then on,
so it and its like are left out.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Puts a database back the way an upgrade found it
feb383e 
Everything needed was written down while the upgrade ran: what each
table looked like, the functions its indexes call, the rows in the
backup_ tables, and what the settings held. This puts them back in the
order they depend on each other, drops the tables the upgrade added,
and notes on the run that it has been undone.

The upgrader's own tables are left alone throughout. They are where the
answer is read from, and a rollback that took them with it could not
record that it had happened.

Nothing outside the database is touched.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Lets the recorded SQL past the checks meant for assembled queries
fa71362 
The database layer refuses a query holding a quote, and refuses one
holding a semicolon whatever else it is told, so a recorded CREATE TABLE
and the body of a recorded function were both turned away. Neither is
built around anything supplied from outside: they are the database's own
account of itself, read back.

The installer and the migrations turn the same checks off around their
own DDL. This puts the setting back as it found it.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Finds the tables when the prefix names the database too
e8d9070 
On MySQL the prefix can be `smf`.smf_ while list_tables() answers with
bare names, so anything comparing one against the other never finds a
table that is sitting right there. The upgrader saw a plain prefix and
recorded everything; a rollback asked for through SSI saw the qualified
one and reported there was nothing to put back.

Table::exists() compares them that way too, which is worth knowing about
separately.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Reports what a rollback could not run, and finds the tables to run it on
ca0ba28 
A prefix that names the database means nothing ever selected one, since
every query SMF makes says which database it means. The recorded SQL
does not, having been written while the prefix was plain, so on MySQL
every statement was refused with 'No database selected' and the rollback
reported itself done having restored nothing.

Errors are still skipped, so that one bad statement does not abandon a
half restored forum, but what was refused is kept and told to the
caller. Skipped is not the same as unnoticed.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Puts a sequence where recreating it would have started it
f241e1b 
Dropping a table does not drop the sequence feeding it, so a sequence
being put back is already there and asking for it again is refused. All
41 of them were refused on every rollback, and nothing noticed, because
the sequences were already at usable values and the comparison saw
nothing wrong.

What the statement was for is the number it would have started at, and
setval still gives that, whether or not the sequence survived.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Keeps the record of a run whole from the first write to the last
90da295 
Four things kept a run from describing the upgrade that made it.

The list of tables to back up is indexed by substep, and array_filter
leaves a hole where each name it dropped had been. The command line
walks the dense list of substeps instead, so only the browser reached
the gap, where it stopped mid backup on an undefined index.

Config::getCurrentSettings() refuses a Settings.php touched since the
request began. An upgrade writes that file more than once, so from the
second write onwards it was reading its own work and giving up, and
db_character_set and db_mb4 -- the two the rollback most needs -- were
never written down.

The run closed before that last write rather than after it, and the
write then opened a run of its own to record settings into: a row with
no backup behind it, left behind by every upgrade that finished.

The run now opens where the admin presses Continue, so the settings that
step changes have somewhere to go, and closes after the last thing the
upgrade writes.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Covers the splitting a rollback depends on
30e0375 
statements() is the only part of the rollback that needs no database, and
it is the part where a mistake is silent: a function body split on the
semicolons inside it becomes fragments that parse as nothing, and the
index leaning on that function never comes back.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Takes back the routines an upgrade added
dc97399 
A rollback put the tables back and left SMF 3.0's own functions behind,
so a database calling itself 2.1.7 still held group_concat, its final
function and migrate_inet.

Every routine in the public schema is now named in the record, whether
or not it could be written down, since the names are what says which
ones the upgrade went on to add. Those are dropped once the tables are,
aggregates before the plain functions they are built from.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Puts the docblock for id_run above id_run
d47145e 
It was left behind above UNRECORDED_SETTINGS, which has a docblock of its
own, so the constant carried two and the property carried none.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Offers to put the database back when an upgrade did not finish
8e0e139 
An upgrade that stopped part way leaves an admin in front of a forum
that is neither one version nor the other, with two ways out: carry on,
or put things back. Only the second was missing.

The offer is made on the options page, and only for a run that stopped
part way and took a backup, since nothing else has the rows to put back.
An upgrade that finished is not offered: undoing a working forum is
something to go looking for, not something to be suggested. On the
command line the same thing is asked for with --rollback.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Adds a way to exercise the offer to put the database back
8a84d27 
Upgrades a baseline, marks the run as one that did not finish, and asks
the upgrader for --rollback. Timing a kill so that it lands after the
backup and before the end is a race, and what is being checked here is
the offer rather than the interruption, which interrupt-upgrade.sh
already covers.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Starts a new run when upgrading again after a rollback
3e33b26 
A run that has been undone was still being treated as one under way, so
upgrading again picked it up where it left off: the tables it had
already backed up were passed over, and the definitions describing the
database it had already put back were kept. The second attempt ran with
no backup of its own at all.

Being undone finishes a run whatever its finishing time says.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Leaves nothing behind that would misdirect the next upgrade
ff68507 
The rollback runs inside the upgrader, which started while the forum was
on the version it has just been taken off. Stopping at step two, that
process wrote its progress data out saying so, and the next upgrade read
it, believed the work was done and skipped every v2_1 migration on a
database that had just been put back to 2.1. It still said the upgrade
was complete: the indexes kept their old names and twelve tables were
left as 2.1 had them.

Asking for the rollback also opened a run of its own, since writing any
setting asks which run is under way. It copied nothing, and left open
the next upgrade would take it up as unfinished work and pass over the
backup it never made.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Takes the version to go back to from the run, not from memory
0674f6e 
Config::$modSettings was read before the rollback and still names the
version that has just gone, so the progress data was written out saying
the forum was on 3.0 when it had been put back to 2.1, and the next
upgrade skipped every v2_1 migration. The run itself knows what the
forum was on before it touched anything, which is what it is on again.

A run that never reached the backup step describes no table, so noting a
setting or two on the way does not make it one worth keeping.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
Says what to do next once the database is back
043862d 
The database is 2.1 again and the files on disk are still 3.0, so the
forum will not run until one of them is changed. The notice that the
rollback had happened said none of this and offered nowhere to go.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
@albertlast
albertlast force-pushed the 3.0/migration-rollback-offer branch from fa6b0a0 to 043862d Compare September 7, 2026 04:16
@albertlast albertlast mentioned this pull request Sep 7, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Database Installer Localization Language & internationalization PostgreSQL Theme Unit Testing Upgrader

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@albertlast