Page MenuHomePhabricator

Update gb_autoblock_parent_id to use '0' instead of 'null' as the default
Closed, ResolvedPublic2 Estimated Story Points

Description

When working on T374854: Update the GlobalBlockLookup service to support reading and applying global autoblocks, I found that the index that was changed in T376052: Allow autoblocks on IP addresses in the globalblocks table for IP addresses which are already globally blocked does not fully work as intended. This is because the gb_autoblock_parent_id column can be NULL, and MariaDB considers NULL values to not cause unique constraint violations in the same way as other values. This seems potentially different on different DBMS platforms.

Therefore, we need to convert the gb_autoblock_parent_id column to instead have values as 0 instead of NULL. Because the default was set as NULL when the column was created, we will need to make a maintenance script that updates the rows and then perform the schema change. The schema change can be made at a later time if other work is needed to be done first.

Acceptance criteria
  • Update GlobalBlockManager to always write 0 for gb_autoblock_parent_id - done in d3017e1fb07c19aacc1b45b8e8592f34e7f21722
  • Make a maintenance script to update values of NULL for gb_autoblock_parent_id to instead be 0
    • Run this maintenance script on WMF production
  • Change the schema to make the gb_autoblock_parent_id column not nullable

Event Timeline

Dreamy_Jazz moved this task from Unsorted to Change on the Schema-change board.
Dreamy_Jazz set the point value for this task to 2.

Change #1077706 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/GlobalBlocking@master] Update gb_autoblock_parent_id to 0 from NULL

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

Change #1077714 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/GlobalBlocking@master] Change database default for gb_autoblock_parent_id

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

Change #1077706 merged by jenkins-bot:

[mediawiki/extensions/GlobalBlocking@master] Update gb_autoblock_parent_id to 0 from NULL

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

Dreamy_Jazz renamed this task from Update gb_autoblock_parent_id to not use '0' instead of 'null' as the default to Update gb_autoblock_parent_id to use '0' instead of 'null' as the default.Oct 4 2024, 12:48 PM
Dreamy_Jazz added a project: DBA.

What DBAs should do here?

Per https://wikitech.wikimedia.org/wiki/Schema_changes#Workflow_of_a_schema_change:

Please include at least a DBA also, even for changes that may look trivial, as they can provide constructive feedback if warned in time. Sometimes simple schema changes require a lot of preparation for deployment.

The schema change patch is https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GlobalBlocking/+/1077714.

It has my sign-off. It's straightforward. Just a double/triple check to make sure there is no rows left with NULL as the value.

Mentioned in SAL (#wikimedia-operations) [2024-10-08T09:06:31Z] <Dreamy_Jazz> Ran mwscript-k8s --comment="T376340" -- extensions/GlobalBlocking/maintenance/UpdateAutoBlockParentIdColumn.php --wiki=aawikibooks

Mentioned in SAL (#wikimedia-operations) [2024-10-08T09:12:36Z] <Dreamy_Jazz> Maintenance script for T376340 finished

Looks good on the globalblocks table after running it on production:

select count(*) from globalblocks where gb_autoblock_parent_id is null;
+----------+
| count(*) |
+----------+
|        0 |
+----------+

Change #1077714 merged by jenkins-bot:

[mediawiki/extensions/GlobalBlocking@master] Change database default for gb_autoblock_parent_id

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

Dreamy_Jazz updated the task description. (Show Details)

This can probably skip QA. I will file a Schema-change-in-production ticket for the schema changes made here.