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

Bug 1894064: remove migration code #1935

Merged
merged 1 commit into from May 6, 2024

Conversation

rosahbruno
Copy link
Contributor

@rosahbruno rosahbruno commented May 6, 2024

The only documentation that we had of this was in the CHANGELOG, so there are no accompanying doc changes.

Why this existed

We are removing this migration code since it is no longer needed. The reason for this code was to help move client_ids and first_run_dates over from our previous storage solution from when Glean.js was asynchronous. Newer versions of Glean.js use localStorage which allows synchronous read/writes. Glean.js used to use IndexedDB which forced asynchronous read/writes.

Synchronous is necessary so that we can ensure read/writes execute before pages are unloaded. If we start to write to storage async and the page unloads, that process never finishes and we incur data loss.

Why it can go away

The only product that used Glean.js in production that was migrating to our new storage solution was MDN. They have been the only ever consumer of this feature. We have had the migration code running for ~9 months in MDN, so the majority of users would have migrated over their data by now.

We are removing it because of an error we are seeing when submitting the first ping for any user before they have migrated. A ping is submitted missing both the client_id and the first_run_date since they get lost in a race condition. This is causing a lot of schema ingestion errors.

We've run the migration code for long enough and now it is safe to turn it off. This will also stop the large amount of first_run_date errors that we have been seeing.

Pull Request checklist

  • Quality: Make sure this PR builds and runs cleanly.
    • Inside the glean/ folder, run:
      • npm run test Runs all tests
      • npm run lint Runs all linters
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
  • Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work

@rosahbruno rosahbruno force-pushed the 1894064-remove-migration-code branch from ff6e69b to 87fb854 Compare May 6, 2024 17:39
@rosahbruno rosahbruno marked this pull request as ready for review May 6, 2024 17:40
@auto-assign auto-assign bot requested a review from chutten May 6, 2024 17:40
CHANGELOG.md Outdated Show resolved Hide resolved
@rosahbruno rosahbruno force-pushed the 1894064-remove-migration-code branch from 87fb854 to a5c41e9 Compare May 6, 2024 20:53
@rosahbruno rosahbruno merged commit f265d15 into mozilla:main May 6, 2024
1 of 3 checks passed
@rosahbruno rosahbruno deleted the 1894064-remove-migration-code branch May 6, 2024 20:54
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 this pull request may close these issues.

None yet

2 participants