Skip to content

Bug 2055182 - Run database maintenance after migration#3540

Open
badboy wants to merge 1 commit into
mainfrom
run-automatic-database-vacuum-/wpqqroyowzyo
Open

Bug 2055182 - Run database maintenance after migration#3540
badboy wants to merge 1 commit into
mainfrom
run-automatic-database-vacuum-/wpqqroyowzyo

Conversation

@badboy

@badboy badboy commented Jul 15, 2026

Copy link
Copy Markdown
Member

Based on the same maintenance tasks appservices runs on the places DB

@badboy
badboy requested a review from a team as a code owner July 15, 2026 13:28
@badboy
badboy requested review from travis79 and removed request for a team July 15, 2026 13:28

@travis79 travis79 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this would be hard to cover via a test 🤔, oh well, it isn't probably a likely failure point so it should be okay.

@badboy badboy changed the title Bug Bug 2055182 - Run database maintenance after migration Bug 2055182 - Run database maintenance after migration Jul 15, 2026
@badboy
badboy requested a review from travis79 July 16, 2026 12:30
@badboy badboy added the sqlite Any changes to the new SQLite storage backend label Jul 17, 2026
@badboy
badboy force-pushed the run-automatic-database-vacuum-/wpqqroyowzyo branch from 1ec3d23 to bdbf277 Compare July 17, 2026 10:15
Based on the same maintenance tasks appservices runs on the places DB
@badboy
badboy force-pushed the run-automatic-database-vacuum-/wpqqroyowzyo branch from bdbf277 to 66f117d Compare July 17, 2026 10:19
Comment on lines +258 to +262
// Unvacuumed the database is _smaller_, because the migrated data is in the WAL file.
// It's around 20k bytes.
// Vacuumed & checkpointed the WAL transactions are merged into the database.
let vacuumed_database_size = 32768;
assert_eq!(db_file_size, vacuumed_database_size);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be honest I don't like this test. It's way too fragile and will break if we add any other migration metrics.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Yeah, the 32768 here seems magically dangerous. What if you just check to see if it is bigger and didn't use the comparison to a magic/const/whatever? Just enough to show that the transactions have been applied to the database?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sqlite Any changes to the new SQLite storage backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants