effectively: DELETE FROM user_preferences WHERE up_property = 'skin' AND up_value NOT IN Skin::getSkinNames()

> var_dump( Skin::getSkinNames() );
array(4) {
  ["monobook"]=>
  string(8) "MonoBook"
  ["vector"]=>
  string(6) "Vector"
  ["modern"]=>
  string(6) "Modern"
  ["cologneblue"]=>
  string(11) "CologneBlue"
}

But the up_values need lowercasing to match

Wouldn't take too much to write a script to do something like that...

What about the Skinname- (is this just 'no preference'?), Skinname-0, Skinname-1, and Skinname-2? Are the numeric ones interpreted as actual skins? If so, they should be converted over to the names.

(In reply to comment #2)

What about the Skinname- (is this just 'no preference'?), Skinname-0,
Skinname-1, and Skinname-2? Are the numeric ones interpreted as actual
skins?
If so, they should be converted over to the names.

Skinname- was possibly "default" a long time ago. Or a database artifact. In any case, it's invalid and these users are currently getting Vector.

Skinname-[0-2] used to refer to specific skins, they do have a 1:1 mapping. The ones that point to skins that are still around can be converted, others are using Vector and can be removed.

I wrote a script that does all of this...someone should just run it across all wikis :)

Thank you for volunteering, Chad ;).

(In reply to comment #3)

I wrote a script that does all of this...someone should just run it across
all wikis :)

Can you post or link to this?

(In reply to comment #4)

Thank you for volunteering, Chad ;).

Get me a window and I'll be more than happy to.

(In reply to comment #5)

(In reply to comment #3)

I wrote a script that does all of this...someone should just run it across
all wikis :)

Can you post or link to this?

That would be the "cleanupSkinPrefs" script in the WikimediaMaintenance extension. Here you go: https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FWikimediaMaintenance.git/HEAD/cleanupSkinPrefs.php

(In reply to comment #6)

(In reply to comment #5)

(In reply to comment #3)

I wrote a script that does all of this...someone should just run it across
all wikis :)

Can you post or link to this?

That would be the "cleanupSkinPrefs" script in the WikimediaMaintenance
extension. Here you go:
https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FWikimediaMaintenance.
git/HEAD/cleanupSkinPrefs.php

That's not batched though...

(In reply to comment #7)

(In reply to comment #6)

(In reply to comment #5)

(In reply to comment #3)

I wrote a script that does all of this...someone should just run it across
all wikis :)

Can you post or link to this?

That would be the "cleanupSkinPrefs" script in the WikimediaMaintenance
extension. Here you go:
https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FWikimediaMaintenance.
git/HEAD/cleanupSkinPrefs.php

That's not batched though...

Really the only wiki that maybe needs batching is enwiki. The rest have a minuscule number of rows to update.

There's the DELETE part too.. Both should be easily updated to use batched queries

Our delete wrapper in it's current form doesn't take options...

Ideally

			do {
				$res = $dbw->delete(
					'user_properties',
					array(
						'up_property' => 'skin',
						'up_value NOT IN (' . $dbw->makeList( $dontChange ) . ')'
					),
					__METHOD__,
					array( 'LIMIT' => 50 )
				);
				wfWaitForSlaves();
			} while ( $res->numRows() === 50 );

but in the current state... Ugh.

			do {
				$res = $dbw->query(
					"DELETE FROM user_properties WHERE up_property='skin' AND up_value NOT IN ({$dbw->makeList( $dontChange )}) LIMIT 50",
					__METHOD__
				);
				wfWaitForSlaves();
			} while ( $res->numRows() === 50 );

Logging a bug for it!

(In reply to comment #9)

There's the DELETE part too.. Both should be easily updated to use batched
queries

Our delete wrapper in it's current form doesn't take options...

"DELETE ...LIMIT" isn't part of the standard, and it's only supported in MySQL and Sqlite :)

(In reply to comment #6)

That would be the "cleanupSkinPrefs" script in the WikimediaMaintenance
extension. Here you go:
https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FWikimediaMaintenance.git/HEAD/cleanupSkinPrefs.php

Nice to see that there's a script for this (or a start to one). :-)

It's in the wrong place, though. The most natural place to find such a script would be in the maintenance directory of MediaWiki core (this is where I looked after Matt posted his question in comment 6). Other wikis will want to clean out their skin preferences as well, presumably. There's nothing Wikimedia-specific about this, as far as I can tell, so the WikimediaMaintenance extension is simply the wrong place for this script.

For enwiki alone:

mysql> select up_value, count(*) as c from user_properties where up_property = 'skin' group by up_value order by c desc;
+-------------+---------+
| up_value    | c       |
+-------------+---------+
|             | 7183420 |
| monobook    | 2182565 |
| vector      |  162135 |
| cologneblue |   83875 |
| modern      |   76036 |
| 0           |   53927 |
| standard    |   34108 |
| chick       |   31920 |
| nostalgia   |   21110 |
| myskin      |   16184 |
| simple      |   14140 |
| 2           |    3060 |
| 1           |    1752 |
| amethyst    |    1291 |
| minerva     |      39 |
| on          |      16 |
+-------------+---------+
16 rows in set (1 min 8.72 sec)

Could we get a report for all wikis? Would help in gauging how much cleanup to do.

Sure, see tin:~krenair/T54778.out. It's all in PHP serialized objects because sql.php.

Cleaned up all small and medium wikis.

causes T114208: User skin preference for MonoBook changed to default (Vector) on small and medium wikis (about 95% of wikis)

Propose rejecting this outright.

In T54778#1821806, @demon wrote:

Propose rejecting this outright.

Doing it would make reporting easier. E.g. https://phabricator.wikimedia.org/T47179#531339 does not group them in a fully readable way.

However, reporting is still possible without this. It would just need some additional work (T47179: Skin and active editor correlation).

In T54778#1821943, @ori wrote:

The Wikimedia Foundation database team is currently operating as a skeleton crew of one full-time DBA. There are 138 open tasks with the #database tag. The previous attempt to make progress on this task resulted in data loss. TL;DR: no.

I don't see how any of what you write here makes this task declined. Stalled, perhaps. There's still real value in keeping a tidy database (cf. T54777).

In T54778#1822498, @MZMcBride wrote:

I don't see how any of what you write here makes this task declined. Stalled, perhaps. There's still real value in keeping a tidy database (cf. T54777).

Our DBA would disagree. "no it [the cleanup] shouldn't [be done] - dropping rows like this has 0 effect on both disk space and performance due to fragmentation"

So... the disk space concerns on T54777: user_properties table bloat are not a real problem then?

In T54778#1822685, @demon wrote:

In T54778#1822498, @MZMcBride wrote:

I don't see how any of what you write here makes this task declined. Stalled, perhaps. There's still real value in keeping a tidy database (cf. T54777).

Our DBA would disagree. "no it [the cleanup] shouldn't [be done] - dropping rows like this has 0 effect on both disk space and performance due to fragmentation"

For the purposes of this task, I don't care about disk space or performance. Neither are of those were mentioned when filing this task. The rows in user_properties are queried as part of generating a database report. It's possible that we could put skin usage stats in the site_stats table or something.

MediaWiki isn't the only consumer of user_properties, and even if it were, doing database cleanup has as much merit as cleanup/tidying of anything. There's no rush; it should be possible to do this type of cleanup in a safe and methodical way.

It's fine to decline the task if we think it's too hard for the benefit here and now; we can reopen in a brighter future (perhaps one where Phabricator search works! then we'll query for declined blockers of T18660).

In T54778#1823971, @Legoktm wrote:

So... the disk space concerns on T54777: user_properties table bloat are not a real problem then?

This is the important part imho. T54777 was used in the past to block development; if that's not a real concern, please decline that one as well so that we can resume improving MediaWiki's feature without needless worries for the database. :)

I agree that this would be good to finally clean-up. If nothing else, it causes lots of confusion for people trying to analyze our preference stats (like Niharika was doing). The only reason this task was abandoned is that the maintenance script was buggy (https://gerrit.wikimedia.org/r/#/c/242374/) and should probably be rewritten from scratch (and at the time we only had 1 DBA to help with the effort).

What about just a general remove deprecated user preference script that can remove entries for a specified preference with the specified value, and manually running it for each skin that doesn't exist anymore?

In T54778#6266543, @DannyS712 wrote:

What about just a general remove deprecated user preference script that can remove entries for a specified preference with the specified value, and manually running it for each skin that doesn't exist anymore?

Increasing the amount of human input increases the chance of error.

If we're going to do it, I guess we need a list of known bad/invalid values (reviewed and confirmed), and a mapping of what they want replacing with, or if they should just be removed outright because they fallback to the defaults

I want to take a look at this but I'm still a bit confused about T114208. Was it because it was accidentally removing monobook as skin values too (why?) or something more complex was in play. Does anyone remember more details of that?

In T54778#9027127, @Ladsgroup wrote:

I want to take a look at this but I'm still a bit confused about T114208. Was it because it was accidentally removing monobook as skin values too (why?) or something more complex was in play. Does anyone remember more details of that?

IIRC, the script didn't cope with all of the ludicrous multiple different ways we store the skin preference. The original attempt to fix the script lists some improvements, before it was just deleted instead.