Page MenuHomePhabricator
Create Task
Maniphest T225643

Schema change to oathauth_users
Closed, ResolvedPublic
Actions

Description

As part of changes to allow yubikey support, a schema change is needed. At https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/OATHAuth/+/496451/ the following is proposed https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/OATHAuth/+/496451/47/sql/mysql/patch-add_generic_fields.sql :

ALTER TABLE /*_*/oathauth_users
	ADD module VARCHAR( 255 ) NOT NULL,
	ADD data BLOB NULL;

Backwards compatible: Yes, as only new columns to be added (There's a part to remove some no longer used fields as well, but we'd want to do that later so we could revert back in case bad things happen)

Replicate to cloud: No, table should already be blacklisted anyway

Testing: Unable to be tested on "just" testwiki, tested locally on dev machines

When: ASAP

Please note: The oathauth_users table is a global table, so its in the centralauth database, and not all the individual wiki databases for SUL wikis.

Table should be relatively small in terms of number of rows, has a PK that is the id column (FK of user.user_id)

Where:

  • centralauth.oathauth_users
  • labswiki.oathauth_users
  • labtestwiki.oathauth_users

Private:

  • advisorswiki.oathauth_users
  • arbcom_cswiki.oathauth_users
  • arbcom_dewiki.oathauth_users
  • arbcom_enwiki.oathauth_users
  • arbcom_fiwiki.oathauth_users
  • arbcom_nlwiki.oathauth_users
  • auditcomwiki.oathauth_users
  • boardgovcomwiki.oathauth_users
  • boardwiki.oathauth_users
  • chairwiki.oathauth_users
  • chapcomwiki.oathauth_users
  • checkuserwiki.oathauth_users
  • collabwiki.oathauth_users
  • ecwikimedia.oathauth_users
  • electcomwiki.oathauth_users
  • execwiki.oathauth_users
  • fdcwiki.oathauth_users
  • grantswiki.oathauth_users
  • id_internalwikimedia.oathauth_users
  • iegcomwiki.oathauth_users
  • ilwikimedia.oathauth_users
  • internalwiki.oathauth_users
  • legalteamwiki.oathauth_users
  • movementroleswiki.oathauth_users
  • noboard_chapterswikimedia.oathauth_users
  • officewiki.oathauth_users
  • ombudsmenwiki.oathauth_users
  • otrs_wikiwiki.oathauth_users
  • projectcomwiki.oathauth_users
  • searchcomwiki.oathauth_users
  • spcomwiki.oathauth_users
  • stewardwiki.oathauth_users
  • techconductwiki.oathauth_users
  • transitionteamwiki.oathauth_users
  • wg_enwiki.oathauth_users
  • wikimaniateamwiki.oathauth_users
  • zerowiki.oathauth_users

Fishbowl:

  • amwikimedia.oathauth_users
  • cnwikimedia.oathauth_users
  • donatewiki.oathauth_users
  • fixcopyrightwiki.oathauth_users
  • foundationwiki.oathauth_users
  • hiwikimedia.oathauth_users
  • idwikimedia.oathauth_users
  • maiwikimedia.oathauth_users
  • nostalgiawiki.oathauth_users
  • punjabiwikimedia.oathauth_users
  • romdwikimedia.oathauth_users
  • rswikimedia.oathauth_users
  • votewiki.oathauth_users
  • wbwikimedia.oathauth_users

If a DBA could make this change, that would be awesome :)

Related Objects

Event Timeline

Bawolff created this task.Jun 12 2019, 4:34 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 12 2019, 4:34 PM
Bawolff renamed this task from Schema change to oath to Schema change to oathauth_users.Jun 12 2019, 4:34 PM
Marostegui edited projects, added MediaWiki-libs-Rdbms; removed Schema-change-in-production.Jun 12 2019, 4:47 PM
Marostegui moved this task from Triage to Blocked external/Not db team on the DBA board.
Marostegui subscribed.

Can you elaborate a bit more on what you expect to store on those two new columns?
I am removing the Schema-change-in-production tag until the change is merged (https://wikitech.wikimedia.org/wiki/Schema_changes#Workflow_of_a_schema_change)

Legoktm subscribed.Jun 12 2019, 4:58 PM

I think the schema change also needs to be made on private wikis too.

Marostegui added a comment.Jun 12 2019, 5:03 PM

Let's try to follow the template at https://wikitech.wikimedia.org/wiki/Schema_changes#Workflow_of_a_schema_change so it is clear for us (DBAs) where it is needed, once the change is merged and the questions answered.

Reedy subscribed.Jun 12 2019, 8:45 PM
In T225643#5253921, @Marostegui wrote:

Can you elaborate a bit more on what you expect to store on those two new columns?

The first is like the 2fa type/name, the second will be a json blob...

Still all private obviously

In T225643#5253971, @Legoktm wrote:

I think the schema change also needs to be made on private wikis too.

Yup, anywhere oathauth_user exists

Reedy updated the task description. (Show Details)Jun 12 2019, 8:45 PM
Reedy updated the task description. (Show Details)Jun 12 2019, 8:47 PM
Reedy updated the task description. (Show Details)Jun 12 2019, 8:50 PM
Reedy updated the task description. (Show Details)
Reedy updated the task description. (Show Details)Jun 12 2019, 8:55 PM
jcrespo added a comment.Jun 12 2019, 11:24 PM

Just to be clear, before we enter a deadlock :-)- we normally wait for changes to be merged to HEAD (as per deployment policy, all database changes should be "optional" -aka not supposing changes are instant on the cluster), and we wait for the actual merge to be sure the change is final and no major change is likely before deployment. In other words, normally the workflow is Merge to HEAD -> Deployment of schema change to cluster -> Deployment of HEAD feature (or configuration flag enabled, both are ok).

This is a convention we do by default, and we are flexible if the request is reasonable, as normally it avoids last-minute changes ("wait, I forgot we need an extra field"), but I mention it because you seem to be blocked on the schema change deployment for merge, and normally we are blocked on the merge for the schema change production deployment...

Reedy added a comment.Jun 13 2019, 6:40 AM

I’m not saying there won’t be another column (though I’d be surprised), but it was more blocked on some other issues with the code, I haven’t reviewed it recently so not sure of the actual state, Brian should have a better idea

I don’t know if a config flag actually got added, I know we asked a couple of times.. might just need Brian or I to make a change to add it for the transition...

Will have a look and see

jcrespo added a comment.Jun 13 2019, 1:27 PM

Again, just to be clear, we don't require a feature flag (although it is usually nice to have), just that we wait for the code to be final (for some definition of final, normally "merged to head") to deploy the db changes- which cannot be reverted- we can just apply the opposite changes again.

More schema changes is ok, it is nice to ping us about this ("this is phase 1, later we will have phase 2") so we can plan for each iteration.

it was more blocked on some other issues with the code

Based on @Reedy feedback we will wait until HEAD merge and your green light to proceed on production, as based on that I understood the ticket still only a heads up, with the when "TBD".

Last question, is there a limit on the size or insertion rate of the blob column, either implicit or explicit? Not too worried about it, but wondering if it can be a string of arbitrary size inserted as fast as possible?

Reedy added a comment.Jun 13 2019, 5:45 PM
In T225643#5256591, @jcrespo wrote:

Last question, is there a limit on the size or insertion rate of the blob column, either implicit or explicit? Not too worried about it, but wondering if it can be a string of arbitrary size inserted as fast as possible?

I've not seem a limit defined in the code, so I guess whatever mysql enforces (if anything)

What do you mean by inserted as fast as possible?

There's only just over 1000 rows in the centralauth.oathauth_users... I think the only "fast" insertion would be the initial migration run to migrate those all over

Only inserts/updates would be when someone enables 2FA, or changes config etc

Let me know if that answers it, or if I completely missed the point

In T225643#5256591, @jcrespo wrote:

Again, just to be clear, we don't require a feature flag (although it is usually nice to have), just that we wait for the code to be final (for some definition of final, normally "merged to head") to deploy the db changes- which cannot be reverted- we can just apply the opposite changes again.

The question is scheduling then I guess... If we merge to master (as I think it's roughly ready otherwise if we're not adding the feature flag), and can get it deployed before the next branch cut... We're good. I know I'm not a DBA, but I think it's a relatively simple schema change; I'm presuming you might not need to master swap or anything with the number of rows etc

Reedy added a comment.Jun 13 2019, 5:54 PM
In T225643#5257185, @Reedy wrote:
In T225643#5256591, @jcrespo wrote:

Again, just to be clear, we don't require a feature flag (although it is usually nice to have), just that we wait for the code to be final (for some definition of final, normally "merged to head") to deploy the db changes- which cannot be reverted- we can just apply the opposite changes again.

The question is scheduling then I guess... If we merge to master (as I think it's roughly ready otherwise if we're not adding the feature flag), and can get it deployed before the next branch cut... We're good. I know I'm not a DBA, but I think it's a relatively simple schema change; I'm presuming you might not need to master swap or anything with the number of rows etc

To be explicit, this can be merged whenever, but if we're not doing the feature flag, we just need to make sure timings/timelines match up (I know it's offsite next week, so potentially merging it next week after the branch cut might work?)

Marostegui added a comment.Jun 14 2019, 10:50 AM
In T225643#5257185, @Reedy wrote:

The question is scheduling then I guess... If we merge to master (as I think it's roughly ready otherwise if we're not adding the feature flag), and can get it deployed before the next branch cut... We're good. I know I'm not a DBA, but I think it's a relatively simple schema change; I'm presuming you might not need to master swap or anything with the number of rows etc

If the biggest table amongst all the wikis that need this schema change is the one at centralauth, that is just 352K on disk, so it can be done directly on the master some day early in the morning along with the rest of the wikis.
Next week I won't probably be able to do it, but maybe we can aim for the 24th of June week? Would that work? Maybe merge it later next week and I can pick it up on Monday or so?

Reedy added a comment.Jun 14 2019, 12:52 PM

After the branch is cut next week seems reasonable

Marostegui claimed this task.Jun 18 2019, 4:42 AM
Stashbot added a comment.Jun 18 2019, 4:44 AM

Mentioned in SAL (#wikimedia-operations) [2019-06-18T04:44:06Z] <marostegui> Deploy schema change on db1073 (labtestwiki and labswiki) - T225643

Marostegui added a comment.Jun 18 2019, 4:44 AM

I have deployed this change on db1073 for labswiki and labtestwiki just to have it done there in advance to check if something breaks in the next few days.

Marostegui triaged this task as Medium priority.Jun 18 2019, 4:45 AM
Marostegui updated the task description. (Show Details)
Marostegui moved this task from Blocked external/Not db team to In progress on the DBA board.
Reedy moved this task from Backlog to In Progress on the MediaWiki-extensions-OATHAuth board.Jun 18 2019, 10:45 PM
Marostegui added a comment.Jun 19 2019, 12:47 PM

As per my chat with @Reedy the code is merged and he's done some testing and it looks good, so I will try to get this schema change done during this week.

Stashbot added a comment.Jun 19 2019, 12:53 PM

Mentioned in SAL (#wikimedia-operations) [2019-06-19T12:53:08Z] <marostegui> Deploy schema change on the private wikis listed at T225643

Marostegui updated the task description. (Show Details)Jun 19 2019, 1:30 PM

All the private wikis have been altered:

advisorswiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
arbcom_cswiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
arbcom_dewiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
arbcom_enwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
arbcom_fiwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
arbcom_nlwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
auditcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
boardgovcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
boardwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
chairwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
chapcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
checkuserwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
collabwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
ecwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
electcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
execwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
fdcwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
grantswiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
id_internalwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
iegcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
ilwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
internalwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
legalteamwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
movementroleswiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
noboard_chapterswikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
officewiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
ombudsmenwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
otrs_wikiwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
projectcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
searchcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
spcomwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
stewardwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
techconductwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
transitionteamwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
wg_enwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
wikimaniateamwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
zerowiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
jcrespo updated the task description. (Show Details)Jun 19 2019, 1:32 PM
Stashbot added a comment.Jun 20 2019, 5:37 AM

Mentioned in SAL (#wikimedia-operations) [2019-06-20T05:37:29Z] <marostegui> Deploy schema change on centralauth.oathauth_users T225643

Marostegui added a comment.Jun 20 2019, 5:39 AM

centralauth has been done:

root@cumin1001:/home/marostegui# mysql.py -hdb1090:3317 centralauth -e "show create table oathauth_users\G" | egrep "module|data"
  `module` varbinary(255) NOT NULL,
  `data` blob,
Marostegui updated the task description. (Show Details)Jun 20 2019, 5:39 AM
Stashbot added a comment.Jun 20 2019, 10:33 AM

Mentioned in SAL (#wikimedia-operations) [2019-06-20T10:33:39Z] <marostegui> Deploy schema change on the fishbowl wikis list on T225643

Marostegui closed this task as Resolved.Jun 20 2019, 10:39 AM
Marostegui updated the task description. (Show Details)

All the fishbowl wikis are done:

for i in `cat s3_fishbowl  | awk -F "." '{print $1}'`; do echo $i; mysql.py -hdb1123 $i -e "show create table oathauth_users\G" | egrep "module|data";done
amwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
cnwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
donatewiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
fixcopyrightwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
foundationwiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
hiwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
idwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
maiwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
nostalgiawiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
punjabiwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
romdwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
rswikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,
votewiki
  `module` varbinary(255) NOT NULL,
  `data` blob,
wbwikimedia
  `module` varbinary(255) NOT NULL,
  `data` blob,

All the wikis on that list have been altered

Reedy mentioned this in T226826: Drop old oathauth_users columns.Jun 28 2019, 11:45 AM
hashar mentioned this in T228250: PHP Notice: Undefined property: stdClass::$module in OATHAuth/src/OATHUserRepository.php on line 193.Jul 17 2019, 12:53 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