Page MenuHomePhabricator
Create Task
Maniphest T381532

Fix AntiSpoof database schema drifts in production
Open, MediumPublic
Actions

Description

The AntiSpoof extension has a few schema drifts on production:

  • su_name has a default when it should have no default (on at least enwiki db2153)
  • su_error has an incorrect type, it should be text not blob (on at least enwiki db2153)

Related Objects
Search...

Event Timeline

Dreamy_Jazz created this task.Wed, Dec 4, 8:53 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptWed, Dec 4, 8:53 PM
Dreamy_Jazz mentioned this in T313251: Collect and fix schema drifts of wmf-deployed extension tables in production.Wed, Dec 4, 8:56 PM
Dreamy_Jazz updated the task description. (Show Details)Wed, Dec 4, 9:50 PM
Dreamy_Jazz updated the task description. (Show Details)
Ladsgroup subscribed.Wed, Dec 4, 11:57 PM

Can you put the result of the run in a paste or somewhere? :D

Dreamy_Jazz added a comment.EditedThu, Dec 5, 10:12 AM

The result was the following. It seems to have missed the mismatch on su_error (the default isn't detected afaik):

{
    "_metadata": {
        "time_end": 1733353554.2700891,
        "time_start": 1733349098.603314
    }
}
Marostegui subscribed.Tue, Dec 10, 9:37 AM

They all seem to be the same on enwiki:

root@cumin1002:~# /home/marostegui/section s1 | while read host port; do echo "$host:$port"; db-mysql $host:$port enwiki -e "show create table spoofuser\G" | egrep "su_name|su_error" | grep -v KEY ; done
dbstore1008.eqiad.wmnet:3311
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2216.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2212.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2203.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2202.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2188.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2176.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2174.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2173.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2170.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2153.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2146.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2145.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2141.codfw.wmnet:3311
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2130.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db2116.codfw.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1240.eqiad.wmnet:3311
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1239.eqiad.wmnet:3311
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1235.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1234.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1232.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1219.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1218.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1207.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1206.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1196.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1195.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1186.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1184.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1169.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
db1163.eqiad.wmnet:3306
  `su_name` varbinary(255) NOT NULL DEFAULT '',
  `su_error` blob DEFAULT NULL,
Marostegui triaged this task as Medium priority.Tue, Dec 10, 9:38 AM
Marostegui moved this task from Triage to Ready on the DBA board.
Marostegui mentioned this in rSCHCH1b62d0651f50: change_spoofuser_T381532.py: New schema change.Tue, Dec 10, 10:45 AM
Marostegui added a comment.Tue, Dec 10, 11:15 AM

https://gitlab.wikimedia.org/repos/sre/schema-changes/-/blob/main/2024/change_spoofuser_T381532.py?ref_type=heads

Stashbot added a comment.Tue, Dec 10, 2:18 PM

Mentioned in SAL (#wikimedia-operations) [2024-12-10T14:18:21Z] <marostegui@cumin1002> dbctl commit (dc=all): 'Depooling db1169 (T381532)', diff saved to https://phabricator.wikimedia.org/P71678 and previous config saved to /var/cache/conftool/dbconfig/20241210-141820-marostegui.json

Ladsgroup added a comment.Thu, Dec 12, 1:07 PM

Even after you do the schema change, the su_error stays as blob:

cumin2024@db1169.eqiad.wmnet[enwiki]> select * from information_schema.columns where table_schema = 'enwiki' and table_name = 'spoofuser' and column_name = 'su_error'\G
*************************** 1. row ***************************
           TABLE_CATALOG: def
            TABLE_SCHEMA: enwiki
              TABLE_NAME: spoofuser
             COLUMN_NAME: su_error
        ORDINAL_POSITION: 4
          COLUMN_DEFAULT: NULL
             IS_NULLABLE: YES
               DATA_TYPE: blob
CHARACTER_MAXIMUM_LENGTH: 65535
  CHARACTER_OCTET_LENGTH: 65535
       NUMERIC_PRECISION: NULL
           NUMERIC_SCALE: NULL
      DATETIME_PRECISION: NULL
      CHARACTER_SET_NAME: NULL
          COLLATION_NAME: NULL
             COLUMN_TYPE: blob
              COLUMN_KEY: 
                   EXTRA: 
              PRIVILEGES: select,insert,update,references
          COLUMN_COMMENT: 
            IS_GENERATED: NEVER
   GENERATION_EXPRESSION: NULL
1 row in set (0.002 sec)

cumin2024@db1169.eqiad.wmnet[enwiki]> show create table spoofuser;
+-----------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------->
| Table     | Create Table                                                                                                                                                                                        >
+-----------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------->
| spoofuser | CREATE TABLE `spoofuser` (
  `su_name` varbinary(255) NOT NULL,
  `su_normalized` varbinary(255) DEFAULT NULL,
  `su_legal` tinyint(1) DEFAULT NULL,
  `su_error` blob DEFAULT NULL,
  PRIMARY KEY (`su_name`),
  KEY `su_normname_idx` (`su_normalized`,`su_name`)
) ENGINE=InnoDB DEFAULT CHARSET=binary ROW_FORMAT=COMPRESSED |
+-----------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------->
1 row in set (0.001 sec)

P71679#287266 too

I think this is a side-effect of having charset=binary there. Maybe we should use another check?

Marostegui added a comment.Thu, Dec 12, 1:11 PM

Yeah, maybe we can just check using su_name DEFAULT?

Ladsgroup added a comment.Thu, Dec 12, 1:17 PM

Yeah, the check should search for null in column_default I think?
https://wikitech.wikimedia.org/wiki/Auto_schema/examples

cumin2024@db1169.eqiad.wmnet[(none)]> select * from information_schema.columns where table_schema = 'enwiki' and table_name = 'spoofuser' and column_name = 'su_name'\G
*************************** 1. row ***************************
           TABLE_CATALOG: def
            TABLE_SCHEMA: enwiki
              TABLE_NAME: spoofuser
             COLUMN_NAME: su_name
        ORDINAL_POSITION: 1
          COLUMN_DEFAULT: NULL
             IS_NULLABLE: NO
               DATA_TYPE: varbinary
CHARACTER_MAXIMUM_LENGTH: 255
  CHARACTER_OCTET_LENGTH: 255
       NUMERIC_PRECISION: NULL
           NUMERIC_SCALE: NULL
      DATETIME_PRECISION: NULL
      CHARACTER_SET_NAME: NULL
          COLLATION_NAME: NULL
             COLUMN_TYPE: varbinary(255)
              COLUMN_KEY: PRI
                   EXTRA: 
              PRIVILEGES: select,insert,update,references
          COLUMN_COMMENT: 
            IS_GENERATED: NEVER
   GENERATION_EXPRESSION: NULL
1 row in set (0.002 sec)

vs

cumin2024@db1184.eqiad.wmnet[(none)]> select * from information_schema.columns where table_schema = 'enwiki' and table_name = 'spoofuser' and column_name = 'su_name'\G
*************************** 1. row ***************************
           TABLE_CATALOG: def
            TABLE_SCHEMA: enwiki
              TABLE_NAME: spoofuser
             COLUMN_NAME: su_name
        ORDINAL_POSITION: 1
          COLUMN_DEFAULT: ''
             IS_NULLABLE: NO
               DATA_TYPE: varbinary
CHARACTER_MAXIMUM_LENGTH: 255
  CHARACTER_OCTET_LENGTH: 255
       NUMERIC_PRECISION: NULL
           NUMERIC_SCALE: NULL
      DATETIME_PRECISION: NULL
      CHARACTER_SET_NAME: NULL
          COLLATION_NAME: NULL
             COLUMN_TYPE: varbinary(255)
              COLUMN_KEY: PRI
                   EXTRA: 
              PRIVILEGES: select,insert,update,references
          COLUMN_COMMENT: 
            IS_GENERATED: NEVER
   GENERATION_EXPRESSION: NULL
1 row in set (0.002 sec)
Marostegui added a comment.Thu, Dec 12, 1:26 PM
In T381532#10400462, @Ladsgroup wrote:

Yeah, the check should search for null in column_default I think?
https://wikitech.wikimedia.org/wiki/Auto_schema/examples






I believe so yeah, I will change it.
CodeReviewBot added a project: Patch-For-Review.Thu, Dec 12, 1:34 PM
marostegui opened https://gitlab.wikimedia.org/repos/sre/schema-changes/-/merge_requests/28



change_spoofuser_T381532.py: Change the check
Marostegui mentioned this in rSCHCHeaf68951b4e9: change_spoofuser_T381532.py: Change the check.Thu, Dec 12, 1:34 PM
Stashbot added a comment.Thu, Dec 12, 1:36 PM
Mentioned in SAL (#wikimedia-operations) [2024-12-12T13:36:33Z] <marostegui@cumin1002> dbctl commit (dc=all): 'Depooling db1169 (T381532)', diff saved to https://phabricator.wikimedia.org/P71703 and previous config saved to /var/cache/conftool/dbconfig/20241212-133633-marostegui.json
Marostegui added a comment.Thu, Dec 12, 1:49 PM
The check that I pushed to review actually doesn't seem to work:

https://gitlab.wikimedia.org/repos/sre/schema-changes/-/merge_requests/28#note_119908
CodeReviewBot added a comment.Fri, Dec 13, 4:02 PM
marostegui merged https://gitlab.wikimedia.org/repos/sre/schema-changes/-/merge_requests/28



change_spoofuser_T381532.py: Change the check
Marostegui mentioned this in rSCHCH1741e5fc8682: Merge branch 'T381532' into 'main'.Fri, Dec 13, 4:02 PM
Maintenance_bot removed a project: Patch-For-Review.Fri, Dec 13, 4:31 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits