Page MenuHomePhabricator

Split user suppression/hiding (ipb_deleted) out of the ipblocks table
Open, Needs TriagePublic

Description

Problem summary

The ipblocks query review at T346293 highlighted existing tech debt in the area of user suppression. Half the queries for the ipblocks table just want it for ipb_deleted. Suppression and unsuppression are O(N) in the number of contributions, and are sometimes done without deferral. CentralAuth suppression is awkward.

If we split ipb_deleted out to a separate table, and joined on it in all changes lists, we could make local suppression be O(1).

Draft schema

CREATE TABLE actor_deleted (
  ad_id INT UNSIGNED AUTO_INCREMENT NOT NULL,
  ad_actor BIGINT UNSIGNED NOT NULL,
  ad_user INT UNSIGNED,

  PRIMARY KEY(ad_id),
  INDEX ad_actor (ad_actor),
  INDEX ad_user (ad_user)
);

CREATE TABLE ipblocks (
  ...
  ipb_deleted_id INT UNSIGNED,
  ...
);

Discussion

Blocking a user with the "hide user" option causes the user's name to be hidden everywhere. Unblocking such a user causes the user's name to be shown everywhere. In changes (revisions etc.), this is implemented by denormalizing the user-hidden flag into the *_deleted bitfield. In user lists, there is no such denormalization, so everything wanting a user list must join on ipblocks and filter by ipb_deleted.

It's also possible to modify the DELETED_USER bit of the *_deleted bitfield using Special:RevisionDelete. An admin can hide a user's name on a particular revision, although this would be lost if the user were subsequently unhidden using Special:Unblock or Special:CentralAuth.

CentralAuth's cross-wiki suppression is problematic. It inserts a block into every attached wiki, although this can fail due to the unique index (T25310). It also updates the *_deleted bitfields. Unsuppressing needs to delete the block but it only has a heuristic way to find it. Suppression traditionally creates blocks with the auto-block flag enabled, although this may be currently broken (T284873). But this is just an accident of the suppression implementation -- if it is a design requirement for CentralAuth locks to create autoblocks, why is that feature only activated when the account is hidden?

I think user suppression should have its own table where the existence of a row indicates that the user is hidden. Changes lists and user lists would all join on this table. So it wouldn't be necessary to update the bitfields when suppressing or unsuppressing users. This would improve the worst-case performance of user hiding actions and would simplify user lists. It would reduce the number of callers that join on ipblocks and so would simplify the multiblocks migration.

Multiple rows per user would be allowed, and ipblocks would have a foreign key link allowing it to delete its own row.

The behaviour of Special:RevisionDelete could change in future subject to user consultation, but for now I would suggest a disabled checked checkbox when the user is suppressed.

I considered adding a column to the user or actor table, but it would be very sparse, and would add migration complexity due to the expensive schema change. By sparse, I mean a boolean value actor_deleted column would be almost always zero, so a separate table which stores only hidden users would require less storage space than a column.

Event Timeline

I like the idea and the design, my only comment is on the name, a deleted actor might sound similar to a deleted page and sound confusing. I think "actor_suppressed" for table name (or "ipb_name_suppressed") might convey the meaning better?