Skip to content

RFC: Remove obsolete database versions - #22284

Merged
kean merged 2 commits into
trunkfrom
task/remove-migrations
Jan 15, 2024
Merged

RFC: Remove obsolete database versions#22284
kean merged 2 commits into
trunkfrom
task/remove-migrations

Conversation

@kean

@kean kean commented Dec 25, 2023

Copy link
Copy Markdown
Contributor

As surfaced in one of the recent posts by @mokagio, a few tests in the test suite take over a second, which is unacceptable. Fortunately, some are testing obsolete database version migrations from 5+ years ago. Removing these tests reduces the time it takes to run the test by 10 seconds (on M2 Pro).

CoreDataMigrationTests time: 10s → 0.5s (-9.5 s)

This PR only removes the tests, which is a safe call. But I suggest taking it a step further and removing the obsolete database version from 1 to 100. I measured the before and after, and the impact of eliminating these versions is measurable.

App size: 329.2 MB → 323.3 MB (-5.9 MB) (I measure the debug .ipa; the release per-device builds might differ)
MOM Compile time: 7.3 s → 3.0 s (-4.3 s)

Regression Notes

  1. Potential unintended areas of impact: n/a
  2. What I did to test those areas of impact (or what existing automated tests I relied on): n/a
  3. What automated tests I added (or what prevented me from doing so): n/a

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@kean kean added [Type] Task Core Data Issues related to Core Data labels Dec 25, 2023
@kean kean added this to the Pending milestone Dec 25, 2023
@kean
kean requested review from crazytonyli and mokagio December 25, 2023 00:11
@wpmobilebot

wpmobilebot commented Dec 25, 2023

Copy link
Copy Markdown
Contributor
WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22284-171b38f
Version24.0
Bundle IDorg.wordpress.alpha
Commit171b38f
App Center BuildWPiOS - One-Offs #8361
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot

wpmobilebot commented Dec 25, 2023

Copy link
Copy Markdown
Contributor
Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22284-171b38f
Version24.0
Bundle IDcom.jetpack.alpha
Commit171b38f
App Center Buildjetpack-installable-builds #7383
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@kean kean mentioned this pull request Dec 25, 2023
13 tasks
This is necessary to get up to date with a CI configuration change that
requires running on Xcode 15.1.

See #22270
@peril-wordpress-mobile

Copy link
Copy Markdown
Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@wordpress-mobile wordpress-mobile deleted a comment from wpmobilebot Jan 9, 2024
@crazytonyli

Copy link
Copy Markdown
Contributor

What are the impacts of removing old database versions? If customers update an old version of the app that uses the deleted database version to the latest one, I guess the database migration would fail and they would lose their on-device data?

@kean

kean commented Jan 12, 2024

Copy link
Copy Markdown
Contributor Author

If customers update an old version of the app that uses the deleted database version to the latest one, I guess the database migration would fail and they would lose their on-device data?

The number of customers on these versions is likely exceedingly small and can be ignored.

I haven't tested what happens, but it's likely going to lead to a crash. The PR that removes the old versions could tackles this. I think logging the user out and removing all local data would be a good option.

I highly doubt that if someone upgrades from version 85 to 251, it'll go smoothly. Chances are it'll either take forever, or crash, or lead to lost or corrupt data.

@kean
kean merged commit 350bc50 into trunk Jan 15, 2024
@kean
kean deleted the task/remove-migrations branch January 15, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Core Data Issues related to Core Data [Type] Task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants