Closed data-sync-user closed 3 months ago
➤ JR Conlin commented:
Adding a bit of additional context to this bug.
We believe that not all records were properly migrated from AWS to GCP. An unknown number of user records were migrated by setting the user’s current node to Spanner, but also with the replaced_at field set.
The purge_old_records.py script looks to delete old user records from the database. The script determines a user record is “old” by querying ( https://github.com/mozilla-services/syncstorage-rs/blob/b777fa0d967472ca34b023c606cfc5ef5309bf73/tools/tokenserver/database.py#L86 ) the tokenserver database looking for any record that has a replaced_at field set to a timestamp value less than the current time, less a grace_period of between 24 hours (default in purge_old_records.py) and a week ([default|] in the database.py get_old_user_records()) . We will use the force option to ignore node state, which then calls delete_service_data() to delete the service data associated with the old user record, and then deletes the token service record associated with the old user record.
There may exist an edge case where a user’s assigned node was set to Spanner, and the replaced_at datestamp was also set. This would cause all records for a given user to be candidates for deletion.
In token server ( https://github.com/mozilla-services/syncstorage-rs/blob/b777fa0d967472ca34b023c606cfc5ef5309bf73/tokenserver-db/src/models.rs#L357 ) get_or_create_user_sync(), we fetch user records in descending order by creation time. For any user record other than the first result ( https://github.com/mozilla-services/syncstorage-rs/blob/b777fa0d967472ca34b023c606cfc5ef5309bf73/tokenserver-db/src/models.rs#L402 ), we force the replaced_at time to be the latest users creation time. We then capture the first time we’ve “seen” this user (the earliest records created_at timestamp. We then check the most recent user record and if there’s a value in the replaced_at field, or if the node is unassigned, then we return a result ( https://github.com/mozilla-services/syncstorage-rs/blob/b777fa0d967472ca34b023c606cfc5ef5309bf73/tokenserver-db/src/models.rs#L444-L445 ) that forces the replaced_at field to be unset. If the replaced_at is not set, but the assigned node is set, we also ignore ( https://github.com/mozilla-services/syncstorage-rs/blob/b777fa0d967472ca34b023c606cfc5ef5309bf73/tokenserver-db/src/models.rs#L459-L460 ) the set replaced_at value.
The product of this is that IF the most recent user record has a replaced_at value set, then we may accidentally delete that user’s record, even if they’re already assigned to the Spanner node. Our purge script should try to avoid that situation. Ideally, it should also remove the false replaced_at for the most recent user’s record.
The purge_old_records.py script was disabled around the same time (early 2020) as the syncstorage migration from AWS (MySQL based syncstorage nodes) to GCP (Google Cloud Spanner based syncstorage node) began. Users were migrated to Spanner by basically:
replaced_at
value in their user record in the database), which signals:The problem we've encountered with the purge_old_records script is due to: A)) All syncstorage data was copied to the Spanner database, including invalidated data pending deletion by the purge script which was no longer running on either the MySQL nor Spanner nodes. The script was modified in 2022 to detect invalidated data copied to Spanner and issue deletes for them (the "force" option) B) Step #2 of the migration process did not take the purge script actions into account:
The work here is modifying the script to further analyze "replaced" records: the scripts current logic assumes these records point to invalidated syncstorage data when that is not always the case.
┆Issue is synchronized with this Jira Task