Page MenuHomePhabricator

Delete `growthexperiments-mentor-id` properties from user_properties
Closed, ResolvedPublic

Description

Background

In 2021, the Growth-Team moved the mentor/mentee relationship to a separate database table to make a certain type of queries possible/easier. This was done as part of T275773: Move mentor/mentee relationship to a separate database table to make it possible to run more queries on it (Wikimedia specific migration was T279853). We left the growthexperiments-mentor-id rows in user_properties, but those rows are not necessary anymore and can be removed safely.

Problem & Solution

The user_properties table is a large ever-growing table that's already quite large (T54777). Rows identified as not useful should be removed from it as part of housekeeping/maintenance.

Notes

I checked how many rows we would be able to delete, to check whether doing something about mentorship property rows can significantly decrease the size of user_properties. It appears that on some of Growth wikis, the growthexperiments-mentor-id properties represent more than a 1% of user_properties – see below for details.

wikiMentorshipTotal%
arwiki198979144992051.37
viwiki6265250229181.25
kowiki3943634491871.14
cswiki2693625707981.05
frwiki144463222154680.65
bnwiki566621074380.27

(was generated via P22987)

Related Objects

Event Timeline

Restricted Application added subscribers: revi, Aklapper. · View Herald Transcript
Urbanecm_WMF added subscribers: Marostegui, jcrespo.

Hello @Ladsgroup, @Marostegui and @jcrespo, I'm writing this comment to summarize today's IRC discussion. There are two ways how this can be done:

  • Use Amir's script to do DELETE FROM user_properties WHERE up_property='growthexperiments-mentor-id'; in a batched way, with sleeps
  • Write a maintenance script as part of GrowthExperiments, with batching and waits for replication, and run it.

I personally prefer the first option, if that's possible, because it looks like an easier thing to do. However, I'm happy to follow whatever Data-Persistence's recommendation happens to be. @Ladsgroup @Marostegui -- what do you think?

@jcrespo also mentioned that coordinating with how backups are done might be useful. From the Growth end of things, no row with up_property='growthexperiments-mentor-id' is read by GrowthExperiments' code, so I don't think we need to back up the to-be-deleted rows. Of course, accidents can happen and maybe it's a good idea to do the deletions right after a backup of user_properties is taken, to ensure we're able to recover should something go terribly wrong. @jcrespo, can you clarify the needs you'd have from the backup end of things? Thanks!

In case I understood anything from the IRC conversation incorrectly, please feel free to correct me.

@jcrespo, can you clarify the needs you'd have from the backup end of things? Thanks!

Mostly, awareness of when the batch deletion starts to make sure we are ready to recover in case an accident happens. Nothing really special.

The other reason would be to be aware of potential size reductions in tables- those are monitored on every backup, so an otherwise unexplained reduction in size may alert of a potential data loss (probably wouldn't be the case with the relatively small amount of rows to purge).

Of course, accidents can happen and maybe it's a good idea to do the deletions right after a backup of user_properties is taken, to ensure we're able to recover should something go terribly wrong

Yes, that too- having custom backups (beyond the regular ones) of only a subset of records take very little time and can save time. If you warn me with enough time in advance I can do a custom backup of the user_properties table just in case :-)

Question, are the wikis mentioned on the header the only ones "affected" (e.g. because a special config), or were just sampled among all wikis that can potentially have these kind of rows?

As I mentioned on IRC, I would prefer if we do not connect directly to the masters and run stuff in there directly. I would rather get that done via a maintenance script.
The amount of rows that need to be deleted are quite a lot, and I think connecting to the masters' prompt and running commands there manually is very error prone and we can generate incidents (ie: wrong batching, DELETE without WHERE clause etc).
Let's use MW in between if we can and avoid running SQL commands directly. I know it will take longer as the script needs to be written, but better be safe than sorry!

As I mentioned on IRC, I would prefer if we do not connect directly to the masters and run stuff in there directly. I would rather get that done via a maintenance script.
The amount of rows that need to be deleted are quite a lot, and I think connecting to the masters' prompt and running commands there manually is very error prone and we can generate incidents (ie: wrong batching, DELETE without WHERE clause etc).
Let's use MW in between if we can and avoid running SQL commands directly. I know it will take longer as the script needs to be written, but better be safe than sorry!

+1 to a maintenance script. We'll want to clean up other properties later, too, and it would be good to have a code review process for doing this.

@jcrespo, can you clarify the needs you'd have from the backup end of things? Thanks!

Mostly, awareness of when the batch deletion starts to make sure we are ready to recover in case an accident happens. Nothing really special.

The other reason would be to be aware of potential size reductions in tables- those are monitored on every backup, so an otherwise unexplained reduction in size may alert of a potential data loss (probably wouldn't be the case with the relatively small amount of rows to purge).

Ack, makes sense.

Of course, accidents can happen and maybe it's a good idea to do the deletions right after a backup of user_properties is taken, to ensure we're able to recover should something go terribly wrong

Yes, that too- having custom backups (beyond the regular ones) of only a subset of records take very little time and can save time. If you warn me with enough time in advance I can do a custom backup of the user_properties table just in case :-)

Thanks for the offer. Can you please clarify what does "enough time in advance" mean?

Question, are the wikis mentioned on the header the only ones "affected" (e.g. because a special config), or were just sampled among all wikis that can potentially have these kind of rows?

The description has only a couple of affected wikis. It's not a random sample though – it's a list of wikis where the Growth features are deployed the longest and I expect them to have the most rows.

The purge will happen at all wikis that have GrowthExperiments installed (dblist). The majority of wikis will have no such rows, either because deployment there happened post-migration (T279853) or because the wiki has no mentors available. I can check the number of rows at all wikis if that'd be useful.

As I mentioned on IRC, I would prefer if we do not connect directly to the masters and run stuff in there directly. I would rather get that done via a maintenance script.
[...]

+1 to a maintenance script. We'll want to clean up other properties later, too, and it would be good to have a code review process for doing this.

In that case, a maintenance script it is. I don't think we'd be able to write a generic one though – the other cleanups (T304538) will have different rules than a simple "delete them all".

Can you please clarify what does "enough time in advance" mean?

A couple of hours during my working hours- eg. usually the day before on a normal working day.

Change 773488 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] maintenance: Add script to remove growthexperiments-mentor-id properties

https://gerrit.wikimedia.org/r/773488

Change 773488 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] maintenance: Add script to remove growthexperiments-mentor-id properties

https://gerrit.wikimedia.org/r/773488

Mentioned in SAL (#wikimedia-releng) [2022-04-07T05:54:12Z] <urbanecm> deployment-prep: mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki={enwiki,cswiki} --delete # T304461

I verified this change at the beta cluster for cswiki and enwiki:

urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=cswiki
Would delete 199 rows.
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ sql cswiki
wikiadmin@172.16.6.39(cswiki)> select count(*) from user_properties where up_property='growthexperiments-mentor-id';
+----------+
| count(*) |
+----------+
|      199 |
+----------+
1 row in set (0.001 sec)

wikiadmin@172.16.6.39(cswiki)> Bye
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=cswiki --delete
Done! Deleted 199 rows.
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ sql cswiki
wikiadmin@172.16.6.39(cswiki)> select count(*) from user_properties where up_property='growthexperiments-mentor-id';
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (0.001 sec)

wikiadmin@172.16.6.39(cswiki)> Bye
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=enwiki
Would delete 33163 rows.
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ sql enwiki
wikiadmin@172.16.6.39(enwiki)> select count(*) from user_properties where up_property='growthexperiments-mentor-id';
+----------+
| count(*) |
+----------+
|    33163 |
+----------+
1 row in set (0.021 sec)

wikiadmin@172.16.6.39(enwiki)> Bye
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=enwiki --delete
Done! Deleted 33163 rows.
urbanecm@deployment-mwmaint02:/srv/mediawiki/php-master$ sql enwiki

wikiadmin@172.16.6.39(enwiki)> select count(*) from user_properties where up_property='growthexperiments-mentor-id';
+----------+
| count(*) |
+----------+
|        0 |
+----------+
1 row in set (0.002 sec)

wikiadmin@172.16.6.39(enwiki)> Bye

Running it all of beta now.

Can you please clarify what does "enough time in advance" mean?

A couple of hours during my working hours- eg. usually the day before on a normal working day.

Hello! As a heads-up, now that the script is merged, I plan to do the deletion in production the next-next week (week of April 18). I'll send you another heads-up at least a business day before I actually do it, so there's enough time to prepare from your side.

I think the ticket has all the information needed, but I'm summarizing it below anyway:

  • Wikis affected: growthexperiments.dblist
  • Table affected: user_properties
  • Rows affected: WHERE up_property='growthexperiments-mentor-id'
  • Number of rows: Majority of the wikis won't have any, a (biased?) sample is in the description
  • Maintenance script used: T304461.php

Mentioned in SAL (#wikimedia-releng) [2022-04-07T06:07:30Z] <urbanecm> deployment-prep: foreachwiki extensions/GrowthExperiments/maintenance/T304461.php --delete # T304461, output is at P24204

Hello! As a heads-up, now that the script is merged, I plan to do the deletion in production the next-next week (week of April 18). I'll send you another heads-up at least a business day before I actually do it, so there's enough time to prepare from your side.

Thanks. I will be off on Monday and Friday that week. Tuesday after 9UTC would be the ideal time as we do regular backups that day, but I can accommodate to any other day.

Hello! As a heads-up, now that the script is merged, I plan to do the deletion in production the next-next week (week of April 18). I'll send you another heads-up at least a business day before I actually do it, so there's enough time to prepare from your side.

Thanks. I will be off on Monday and Friday that week. Tuesday after 9UTC would be the ideal time as we do regular backups that day, but I can accommodate to any other day.

Tuesday it is then :). Putting it into my calendar.

Just a reminder that backups of all mediawiki metadata dbs ran already earlier today, including the user_properties table.

Mentioned in SAL (#wikimedia-operations) [2022-04-19T16:21:51Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=cswiki --delete # T304461

Mentioned in SAL (#wikimedia-operations) [2022-04-19T16:23:40Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=kowiki --delete # T304461

Mentioned in SAL (#wikimedia-operations) [2022-04-19T17:36:10Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=bnwiki --delete # T304461

Mentioned in SAL (#wikimedia-operations) [2022-04-19T17:38:05Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=arwiki --delete # T304461

Mentioned in SAL (#wikimedia-operations) [2022-04-19T19:34:54Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=viwiki --delete # T304461

Mentioned in SAL (#wikimedia-operations) [2022-04-19T19:35:50Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ mwscript extensions/GrowthExperiments/maintenance/T304461.php --wiki=frwiki --delete # T304461

Mentioned in SAL (#wikimedia-operations) [2022-04-19T19:39:59Z] <urbanecm> [urbanecm@mwmaint1002 ~]$ foreachwikiindblist growthexperiments extensions/GrowthExperiments/maintenance/T304461.php --delete # T304461

Urbanecm_WMF closed this task as Resolved.EditedApr 19 2022, 8:28 PM
Urbanecm_WMF claimed this task.

The cleanup happened. I closely monitored the first few wikis and i didn't find anything suspicious, so I ran it at all the wikis.

Thanks for the work. The backup being so close to the maintenance (but before it) will still be useful for a couple of months in case we receive user complains, previously unnoticed.

Given the percentage of deleted records is low, a defragmentation may not be needed (DBAs to confirm)- but noting that in other cases, e.g. where 30% of the records are deleted, a followup is normally needed to "recover" disk space.

Change 812408 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] Delete maintenance/T304461.php

https://gerrit.wikimedia.org/r/812408

Change 812408 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Delete maintenance/T304461.php

https://gerrit.wikimedia.org/r/812408