I was planning to move the migration shortly, but will need guidance from MediaWiki engineering on whether it's safe to proceed vs. needing to roll back.

Change #1196499 had a related patch set uploaded (by Scott French; author: Scott French):

[operations/puppet@production] deployment_server: Revert production mediawiki releases to 8.1

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

Change #1196499 merged by Scott French:

[operations/puppet@production] deployment_server: Revert production mediawiki releases to 8.1

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

Out of an abundance of caution, I've reverted the small fraction of traffic we currently have serving on 8.3 back to 8.1.

Per logstash, this has occurred 5 times in total over the last 24h while piloting a small fraction of traffic on 8.3. All of these instances appear to implicate the same code path via PraiseworthyMenteeSuggester.

Before moving forward again, the following will need sorted out:

1. How severe and / or widespread this issue is expected to be.
2. What options are available for fixing (or otherwise mitigating) the apparent incompatibility to unblock the migration.

When the offending code is tracked down, https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Don't_use_built_in_serialization and T161647: RFC: Deprecate using php serialization inside MediaWiki should be justification for rewriting the blob serialization for the cached data.

If I remember properly, the RFC followed issues we have encountered with the visibility of a class property changing. We'd get a public property becoming private, and once unserialized there would be two properties with the same name but different visibility. T156364#2978796 T269396#6668455

At least if I look at DateInterval, they serialize the same way under 8.2.0 - 8.2.29, 8.3.0 - 8.3.26, 8.4.1 - 8.4.13 https://3v4l.org/5nCkj

<?php
var_dump(
serialize(
    new DatePeriod(
        new DateTime("2025-10-15 16:50"),
        new DateInterval("P1D"),
        3
        )
)
);
// string(450) "O:10:"DatePeriod":7:{s:5:"start";O:8:"DateTime":3:{s:4:"date";s:26:"2025-10-15 16:50:00.000000";s:13:"timezone_type";i:3;s:8:"timezone";s:16:"Europe/Amsterdam";}s:7:"current";N;s:3:"end";N;s:8:"interval";O:12:"DateInterval":10:{s:1:"y";i:0;s:1:"m";i:0;s:1:"d";i:1;s:1:"h";i:0;s:1:"i";i:0;s:1:"s";i:0;s:1:"f";d:0;s:6:"invert";i:0;s:4:"days";b:0;s:11:"from_string";b:0;}s:11:"recurrences";i:4;s:18:"include_start_date";b:1;s:16:"include_end_date";b:0;}"

The DateInterval::$have_special_relative property does not show up in the serialization which is an indication it is not part of the public API and must be set internally by PHP. It is also not documented in the public API at https://www.php.net/DateInterval (but could have been removed).

My wild guess is because we have compiled php 8.3 with a version of timelib provided by Debian when PHP otherwise bundles it. PHP 8.2 had timelib 2022.08 https://github.com/php/php-src/commit/c02ac2668535b33d3abcf559f185ca7f0a8b7cd2 If correct, that would be class of bug similar to the issue I have found with PHP 8.1 and PHP 8.3 which were compiled with a version of pcre that is different from the one bundled by upstream PHP (see T398245#11010808 , T400693, T386006). Then it is my birthday today 🎉 and it is late, so I am not going to pursue further :-]

Thanks, @hashar!

So, I'm pretty sure our 8.1 and 8.3 builds are both using the bundled timelib, rather than anything provided by debian - e.g., I can see it being compiled and the resulting object file being pulled into the respective SAPIs in the build logs.

Edit: This is consistent with what @bd808 posts above from php -i output.

From a quick scan of the differences in ext/date/php_date.c between 8.1 and 8.3, it just looks like quite a bit has changed (i.e., in addition to / together with the update to the bundled timelib). See e.g. date_object_get_properties_interval and callees in the DateInterval case, where it's clear that the set of private (i.e., not part of public interface stub) properties have changed.

Also happy birthday! :)

Edit: Since I have the code in front of me, we can take a look at the php_date_period_initialize_from_hash function in the critical path for DatePeriod unserialization in 8.1 vs. 8.3: P83947. There are a couple of things going on there, but the (public) include_end_date property is definitely not going to be present in an 8.1-serialized object, since it and the DatePeriod::INCLUDE_END_DATE options constant were not added until 8.2.

See also the example serializations shown in T407403#11279533.

Changing the 3v4l.org test @hashar did in T407403#11279288 to one that includes legacy versions of PHP so that 8.1 output shows up reproduces the serialization diff between 8.1 and 8.3.

Output for 8.2.0 - 8.2.29, 8.3.0 - 8.3.26, 8.4.1 - 8.4.13:

string(450) "O:10:"DatePeriod":7:{s:5:"start";O:8:"DateTime":3:{s:4:"date";s:26:"2025-10-15 16:50:00.000000";s:13:"timezone_type";i:3;s:8:"timezone";s:16:"Europe/Amsterdam";}s:7:"current";N;s:3:"end";N;s:8:"interval";O:12:"DateInterval":10:{s:1:"y";i:0;s:1:"m";i:0;s:1:"d";i:1;s:1:"h";i:0;s:1:"i";i:0;s:1:"s";i:0;s:1:"f";d:0;s:6:"invert";i:0;s:4:"days";b:0;s:11:"from_string";b:0;}s:11:"recurrences";i:4;s:18:"include_start_date";b:1;s:16:"include_end_date";b:0;}"

Output for 7.1.0 - 7.1.33, 7.2.0 - 7.2.34, 7.3.0 - 7.3.33, 7.4.0 - 7.4.33, 8.0.0 - 8.0.30, 8.1.0 - 8.1.33:

string(590) "O:10:"DatePeriod":6:{s:5:"start";O:8:"DateTime":3:{s:4:"date";s:26:"2025-10-15 16:50:00.000000";s:13:"timezone_type";i:3;s:8:"timezone";s:16:"Europe/Amsterdam";}s:7:"current";N;s:3:"end";N;s:8:"interval";O:12:"DateInterval":16:{s:1:"y";i:0;s:1:"m";i:0;s:1:"d";i:1;s:1:"h";i:0;s:1:"i";i:0;s:1:"s";i:0;s:1:"f";d:0;s:7:"weekday";i:0;s:16:"weekday_behavior";i:0;s:17:"first_last_day_of";i:0;s:6:"invert";i:0;s:4:"days";b:0;s:12:"special_type";i:0;s:14:"special_amount";i:0;s:21:"have_weekday_relative";i:0;s:21:"have_special_relative";i:0;}s:11:"recurrences";i:4;s:18:"include_start_date";b:1;}"

In T407403#11279030, @bd808 wrote:

When the offending code is tracked down, https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Don't_use_built_in_serialization and T161647: RFC: Deprecate using php serialization inside MediaWiki should be justification for rewriting the blob serialization for the cached data.

The problematic behavior seems to be in PraiseworthyMenteeSuggester.php caused by storing a list of objects in $this->globalCache (a BagOStuff instance) via refreshPraiseworthyMenteesForMentor and then fetching this list back in getPraiseworthyMenteesForMentor.

The GrowthExperiments\UserImpact\UserImpact class implements JsonSerializable. This keeps SQLBagOStuff from logging a warning about the list of custom objects passed to set. Unfortunately it appears that ultimately MediumSpecificBagOStuff.php still calls serialize when storing values rather than calling json_encode to take advantage of the JsonSerializable storage format. It is actually not clear to me why MediumSpecificBagOStuff thinks that JsonSerializable objects are safe in general with it's current implementations of MediumSpecificBagOStuff::serialize and MediumSpecificBagOStuff::unserialize.

I think a fix might be as simple as adding json_encode on the way into the BagOStuff and json_decode on the way out. This would need to be followed by rebuilding or purging the existing cached values.

https://packagist.org/packages/wikimedia/json-codec (aka the "JsonCodec" service) provides mechanisms for encoding/decoding "third party" objects which don't natively support JSON serialization. MediumSpecificBagOStuff should probably switch to using the JsonCodec service by default instead of serialize/unserialize, but if you need a way to make DateInterval "codecable" that can be done via JsonCodec::addCodecFor( DateInterval::class, ...)

The issue seems specific to DatePeriod. DateInterval also serializes differently between versions, and you get a ton of deprecation warnings when trying to unserialize a PHP 8.1 DateInterval in 8.3, but it does work. 8.3 -> 8.1 works as well. And the only difference in DatePeriod seems to be the presence of the include_end_date property in 8.2+.

So I guess a horrible hack would be to just poke into the serialized format directly: https://3v4l.org/HHYIQ#v8.3.26

In T407403#11279689, @bd808 wrote:

I think a fix might be as simple as adding json_encode on the way into the BagOStuff and json_decode on the way out. This would need to be followed by rebuilding or purging the existing cached values.

JsonSerializable is one-way - json_decode() will just return a PHP array with no indication of what object is being represented. UserImpact has a newFromJsonArray() method, but something would have to call it.

Also in general JSON does not round-trip as it can't differentiate between PHP arrays and stdClass objects.

Using JSON encoding when the class is JsonCodecable would work. UserImpact would have to implement JsonCodecable, which is trivial (it already has the relevant logic). MediumSpecificBagOStuff would have to be fixed to check for JsonCodecable instead of JsonSerializable, use json_encode() if the object to be serialized is JsonCodecable or an array with only scalar or JsonCodecable leaves, and use json_decode() if the cached data looks like a JSON string (that's safe as serialized strings never look like JSON strings).

In T407403#11279725, @cscott wrote:

https://packagist.org/packages/wikimedia/json-codec (aka the "JsonCodec" service) provides mechanisms for encoding/decoding "third party" objects which don't natively support JSON serialization.

That's not really helpful here since we are already dealing with serialized data.

In T407403#11283386, @Tgr wrote:

In T407403#11279689, @bd808 wrote:

I think a fix might be as simple as adding json_encode on the way into the BagOStuff and json_decode on the way out. This would need to be followed by rebuilding or purging the existing cached values.

JsonSerializable is one-way - json_decode() will just return a PHP array with no indication of what object is being represented. UserImpact has a newFromJsonArray() method, but something would have to call it.

Also in general JSON does not round-trip as it can't differentiate between PHP arrays and stdClass objects.

That's where JsonCodec comes in, because it does provide indication for what object is being represented. Thus newFromJsonArray() is called internally when decoding. And it does differentiate between PHP arrays and stdClass objects (=> {"foo":"bar","_type_":"stdClass","_complex_":true}).

The main question for me would be: how do we figure out in a performant way whether a value in cache needs unserialize( $stringValueLoadedFromCache ) or $codec->newFromJsonString( $stringValueLoadedFromCache )? We could try both and return which doesn't crash, but that's probably not ideal...

That being said, the fastest way to unblock this task is to preempt the upstream work in core's caching logic and to do the json serialization in PraiseworthyMenteeSuggester for now. I'll make UserImpact implement JsonCodecable for that, and so it will be ready once *BagOStuff is.

Change #1196872 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/GrowthExperiments@master] refactor(UserImpact): Improve serializability

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

Change #1196873 had a related patch set uploaded (by Michael Große; author: Michael Große):

[mediawiki/extensions/GrowthExperiments@master] fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration

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

In T407403#11284486, @Michael wrote:

That's where JsonCodec comes in, because it does provide indication for what object is being represented. Thus newFromJsonArray() is called internally when decoding. And it does differentiate between PHP arrays and stdClass objects (=> {"foo":"bar","_type_":"stdClass","_complex_":true}).

Yes but that requires classes to be JsonCodecable (or specifically known to JsonCodec, like stdClass is). Trying to JSON-serialize classes that did not sign up for it opens you up to a world of hurt (what if it did something important in __sleep()? what if it contains a PHP resource? etc)

We could add plugins to JsonCodec for common PHP internal classes like datetime-related objects, but that's not solving any problem that we actually have - MediumSpecificBagOStuff would already emit a warning if it had to serialize such an object. (We do have such warnings but not that many and not for standard PHP classes - here's an ad hoc dashboard.)

The main question for me would be: how do we figure out in a performant way whether a value in cache needs unserialize( $stringValueLoadedFromCache ) or $codec->newFromJsonString( $stringValueLoadedFromCache )? We could try both and return which doesn't crash, but that's probably not ideal...

That part is easy. If it starts with { it is a JSON string. If it starts with a letter it is a PHP serialization. If it starts with a number, it's a raw scalar value. There shouldn't be any other possibilities.

I'll make UserImpact implement JsonCodecable for that, and so it will be ready once *BagOStuff is.

FWIW task-related GowthExperiments classes are already JsonCodecable; it would be nice to convert the rest of the JsonSerializable classes, it tends to be a trivial amount of work.

Change #1196872 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] refactor(UserImpact): Improve serializability

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

Change #1196873 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration

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

I don't think there can be QA for this other than rolling it out to production, turning PHP 8.3 on again and looking at the logs. If the logs are fine and this error does not show up again, then this task can just be closed.

Change #1197866 had a related patch set uploaded (by Krinkle; author: Michael Große):

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.24] fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration

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

@Michael It missed the train by a few hours, so I've proposed a backport in the interest of not delaying the PHP 8.3 upgrade by an extra week. I can find a time for it, but feel free to take it if you can. If you'd like me to roll it out, could you describe briefly how to "naturally" trigger this code path from the web?

In T407403#11297163, @Krinkle wrote:

@Michael It missed the train by a few hours, so I've proposed a backport in the interest of not delaying the PHP 8.3 upgrade by an extra week. I can find a time for it, but feel free to take it if you can. If you'd like me to roll it out, could you describe briefly how to "naturally" trigger this code path from the web?

@Krinkle This is a bit tricky to test, I'm afraid. One needs to be a mentor and have at least some mentees that are being considered "praise worthy" -they have done enough edits recently, thresholds vary by wiki- for the relevant code-paths to be touched. Maybe @Urbanecm_WMF has a better idea for how to test it. Otherwise, I'm not sure if there is an alternative to just back-porting it, watching the logs and crossing one's fingers.

Change #1197866 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.24] fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration

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

Mentioned in SAL (#wikimedia-operations) [2025-10-22T20:14:09Z] <krinkle@deploy2002> Started scap sync-world: Backport for [[gerrit:1197866|fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration (T407403)]]

You should be able to just run

$u = MW::user( 'SomeMentor' );
$gepms = MW::srv()->get( 'GrowthExperimentsPraiseworthyMenteeSuggester' );
$gepms->getPraiseworthyMenteesForMentor( $u );

Finding a mentor with praisworthy mentees is a bit of a moving target (when someone is praised they stop being praiseworthy). You can go to e.g. https://cs.wikipedia.org/wiki/Special:ManageMentors and pick usernames until you find someone (I think enwiki has this feature disabled?).

To test the write-new / read-new path, I think you can just delete the cache for some mentor

sudo $gepms->globalCache->delete( $gepms->makeCacheKeyForMentor( $u );
$gepms->refreshPraiseworthyMenteesForMentor( $u );
$gepms->getPraiseworthyMenteesForMentor( $u );

Once the patch is deployed, the old entries should expire in a day.

Mentioned in SAL (#wikimedia-operations) [2025-10-22T20:18:30Z] <krinkle@deploy2002> krinkle: Backport for [[gerrit:1197866|fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration (T407403)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.

Mentioned in SAL (#wikimedia-operations) [2025-10-22T20:26:47Z] <krinkle@deploy2002> Finished scap sync-world: Backport for [[gerrit:1197866|fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration (T407403)]] (duration: 12m 38s)

Change #1198346 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/GrowthExperiments@master] MentorDashboard,UserImpact: bump cache version and set proper keygroup

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

Change #1198389 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.24] MentorDashboard,UserImpact: bump cache version and set proper keygroup

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

Change #1198395 had a related patch set uploaded (by Krinkle; author: Michael Große):

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.23] fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration

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

Change #1198396 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.23] MentorDashboard,UserImpact: bump cache version and set proper keygroup

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

Change #1198395 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.23] fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration

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

Mentioned in SAL (#wikimedia-operations) [2025-10-23T20:45:39Z] <krinkle@deploy2002> Started scap sync-world: Backport for [[gerrit:1198395|fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration (T407403)]]

Mentioned in SAL (#wikimedia-operations) [2025-10-23T20:49:38Z] <krinkle@deploy2002> krinkle: Backport for [[gerrit:1198395|fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration (T407403)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.

Mentioned in SAL (#wikimedia-operations) [2025-10-23T21:00:44Z] <krinkle@deploy2002> Finished scap sync-world: Backport for [[gerrit:1198395|fix(MentorDashboard): fix caching for PHP 8.1 -> 8.3 migration (T407403)]] (duration: 15m 06s)

Change #1198389 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.24] MentorDashboard,UserImpact: Bump cache and set proper keygroup

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

Change #1198396 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.45.0-wmf.23] MentorDashboard,UserImpact: Bump cache and set proper keygroup

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

Mentioned in SAL (#wikimedia-operations) [2025-10-23T21:31:27Z] <krinkle@deploy2002> Started scap sync-world: Backport for [[gerrit:1198389|MentorDashboard,UserImpact: Bump cache and set proper keygroup (T407403)]], [[gerrit:1198396|MentorDashboard,UserImpact: Bump cache and set proper keygroup (T407403)]]

Mentioned in SAL (#wikimedia-operations) [2025-10-23T21:33:30Z] <krinkle@deploy2002> krinkle: Backport for [[gerrit:1198389|MentorDashboard,UserImpact: Bump cache and set proper keygroup (T407403)]], [[gerrit:1198396|MentorDashboard,UserImpact: Bump cache and set proper keygroup (T407403)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.

Mentioned in SAL (#wikimedia-operations) [2025-10-23T21:42:01Z] <krinkle@deploy2002> Finished scap sync-world: Backport for [[gerrit:1198389|MentorDashboard,UserImpact: Bump cache and set proper keygroup (T407403)]], [[gerrit:1198396|MentorDashboard,UserImpact: Bump cache and set proper keygroup (T407403)]] (duration: 10m 34s)

Change #1198346 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] MentorDashboard,UserImpact: Bump cache and set proper keygroup

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