Change 831914 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add maintenance script to update UTC timestamps of events

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

@Ladsgroup Hi! I have a question for you and DBAs in general. For this task, we want to write a maintenance script that periodically changes data in a table, recomputing it from the other fields. Basically, for each row, it reads the local time, reconverts it to UTC, and stores it if it's different from the UTC value that was previously stored. The code I have right now (patch above) runs in batches of 500 rows, by using the primary key, does a SELECT on the master DB, recomputes the values for all rows, and then runs an INSERT ODKU (via upsert) to update the rows if at least one of them needs to be updated. So far so good, I think (?).

My question is what precautions we should take against concurrent updates, i.e., someone changing a record from the interface while the script runs. I was thinking about starting an explicit transaction and using FOR UPDATE or LOCK IN SHARE MODE, but since the script scans the whole table, I think we may risk locking too many rows; SQLite in particular locks the whole table. I'm also not sure about the specific methods to use for transactions (begin() vs startAtomic()), and if we need to do anything special given it's a maintenance script (so different transaction rules, I think).

For completeness, the scenario that I want to avoid is the following (simplified example):

• An event has local time = 17:00, and UTC time = 16:00
• The script reads the row and determines that the UTC time should be changed to 15:00
• Meanwhile, someone edits the event using the interface, setting local time = 10:00. At this point it doesn't matter if the new UTC time is 9:00 or 8:00, because it's different from 15:00 in any case.
• Then the script updates the row, setting UTC time to 15:00, but the local time is now 10:00, and the two are out of sync

I recognize this is quite an edge case. If transactions/locks would be overkill, perhaps I can change the write query to be something like:

SET utc_time = ( CASE WHEN local_time = $localTimeFromBefore THEN $newUTCTime ELSE $oldUTCTime END )

thus effectively skipping the row, in the hope that it'll be updated the next time around. However, I would like to better understand the transaction option first.

Thanks in advance!

We discussed this at the engineering meeting today. Since concurrent updates would be a super-rare edge case, we're going with the SET+CASE option for now.

Also, as @cmelo noted, the "skipped" rows may not even have to be updated on the next run. Let's say that the script determines that a record should be changed, AND someone is updating it at the same time. Since both the script and the web request are using the most up-to-date time zone rules, any change that the script might have made will also be applied upon submitting the form in the web request, so the final UTC timestamp is always gonna use the most recent time zone rules.

Change 831914 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add maintenance script to update UTC timestamps of events

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

Documented. Language tweaks are welcome.

@vaughnwalters You will have to test this locally as explained in the documentation. Main things to test are that the script is updating time zones that should be updated, and not updating time zones that should not be updated. Also the --timezone parameter.

@Daimona see the attached image:

I have run this script and it successfully ran, but it didn't update anything, presumably because no time zones need updated. I am not sure how to test this more fully, or if this really can't be tested until time zones actually do need updated?

Here is the an entry from before running the script, and it looks the same after running the script, as it should since the time zone doesn't need updated.

In T316687#8440007, @vaughnwalters wrote:

I have run this script and it successfully ran, but it didn't update anything, presumably because no time zones need updated. I am not sure how to test this more fully, or if this really can't be tested until time zones actually do need updated?

Ahh yes, good point. Indeed, assuming you didn't do anything special, time zones in your database likely don't need to be updated. I think the easiest way to test this would be to manually change the stored data so that it's wrong. In particular, you would have to change event_start_utc and event_end_utc to be something other than the expected values based on event_start_local and event_end_local, and then run the script and ensure that they were corrected.

This is working correctly now.

✅ Script is created and can be run from the command line
✅ Script is documented in the extension manual

before:

after:

@Daimona one small note for on the docs page:
https://www.mediawiki.org/wiki/Extension:CampaignEvents#Script_to_update_timezones
the php script should be run in the extensions folder not extension

In T316687#8443649, @vaughnwalters wrote:

@Daimona one small note for on the docs page:
https://www.mediawiki.org/wiki/Extension:CampaignEvents#Script_to_update_timezones
the php script should be run in the extensions folder not extension

Right, just fixed that, thank you.

@Daimona one more thing, I am not able to get this to work using the optional --timezone parameter.

root@9ee4cc153b75:/var/www/html/w# php extensions/CampaignEvents/maintenance/UpdateUTCTimestamps.php --timezone +01:00
Updating UTC timestamps in the campaign_events table...
[71bad06078bbe91d1f37bd75] [no req]   TypeError: Argument 3 passed to MediaWiki\Extension\CampaignEvents\Maintenance\UpdateUTCTimestamps::updateBatch() must be of the type string or null, array given, called in /var/www/html/w/extensions/CampaignEvents/maintenance/UpdateUTCTimestamps.php on line 66
etc etc

root@9ee4cc153b75:/var/www/html/w# php extensions/CampaignEvents/maintenance/UpdateUTCTimestamps.php --timezone America/Tijuana
Updating UTC timestamps in the campaign_events table...
[dd0a95b2c1e0c22f4d91dcec] [no req]   TypeError: Argument 3 passed to MediaWiki\Extension\CampaignEvents\Maintenance\UpdateUTCTimestamps::updateBatch() must be of the type string or null, array given, called in /var/www/html/w/extensions/CampaignEvents/maintenance/UpdateUTCTimestamps.php on line 66

and then if I run it without the timezone param it works correctly:

root@9ee4cc153b75:/var/www/html/w# php extensions/CampaignEvents/maintenance/UpdateUTCTimestamps.php
Updating UTC timestamps in the campaign_events table...
Batch 0-500: ~1 updated.
Batch 500-1000: 0 updated.
Done.

Change 864874 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Fix typehint in the timezone script

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

In T316687#8444892, @vaughnwalters wrote:

@Daimona one more thing, I am not able to get this to work using the optional --timezone parameter.

Yup, confirmed, fix pushed.

Change 864874 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Fix typehint in the timezone script

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

timezone param is working correctly now:

@Daimona if there is a string that doesn't match a time zone, it now fails silently. It used to throw a Unknown or bad timezone error, so am wondering should there be an error thrown here or will this suffice?

The only indicator that it is not a valid timezone, is that for invalid timezones it doesn't display the second line Batch 500-1000: 0 updated. in the string that is shown. For valid timezones, it will show a batch of 500-1000 even if there are no updates to that batch.

In T316687#8482173, @vaughnwalters wrote:

@Daimona if there is a string that doesn't match a time zone, it now fails silently. It used to throw a Unknown or bad timezone error, so am wondering should there be an error thrown here or will this suffice?

I'm actually a bit surprised to read this, because the script never tried to validate the timezone; it simply uses it in a query. I'm gonna implement validation anyway.

The only indicator that it is not a valid timezone, is that for invalid timezones it doesn't display the second line Batch 500-1000: 0 updated. in the string that is shown. For valid timezones, it will show a batch of 500-1000 even if there are no updates to that batch.

This is actually a consequence of another bug where we stop early if no events with the given timezone exist, which I'm also going to fix.

Change 870965 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/CampaignEvents@master] Add tz validation and fix batching in UpdateUTCTimestamps

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

Change 870965 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Add tz validation and fix batching in UpdateUTCTimestamps

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

The bugs reported in T316687#8482173 should now be fixed.

The maintenance script is no longer working on my local. If I change UTC or local times, the query still returns the same results before or after running the script.

In T316687#8545385, @vaughnwalters wrote:

The maintenance script is no longer working on my local. If I change UTC or local times, the query still returns the same results before or after running the script.

I can't reproduce this. I created an event using the same timezone and times as your first screenshot:

sqlite> select event_start_local,event_start_utc,event_end_local,event_end_utc from campaign_events where event_id=138;
event_start_local  event_start_utc  event_end_local  event_end_utc 
-----------------  ---------------  ---------------  --------------
20230414160001     20240414170301   20250414160033   20240414270004

then run the script:

I have no name!@6f2284f085f8:/var/www/html/w$ php extensions/CampaignEvents/maintenance/UpdateUTCTimestamps.php 

*******************************************************************************
NOTE: Do not run maintenance scripts directly, use maintenance/run.php instead!
      Running scripts directly has been deprecated in MediaWiki 1.40.
      It may not work for some (or any) scripts in the future.
*******************************************************************************

Updating UTC timestamps in the campaign_events table...
Batch 0-500: ~1 updated.
Done.

and then checked the DB:

sqlite> select event_start_local,event_start_utc,event_end_local,event_end_utc from campaign_events where event_id=138;
event_start_local  event_start_utc  event_end_local  event_end_utc 
-----------------  ---------------  ---------------  --------------
20230414160001     20230414210001   20250414160033   20250414210033

so it worked as expected.

@vaughnwalters Can you try again with a new event, listing here all the steps that you followed?