Page MenuHomePhabricator

DBA review: new table for associating edits and events
Closed, ResolvedPublic

Description

Context: For T378035: [EPIC] Collaborative contributions MVP, we want to implement a feature where event participants can indicate that an edit was made as part of an event. To do that, after someone makes an edit, we will check whether they are participating in an event that is currently ongoing and targeting the current wiki (information available via the other CampaignEvents table). If so, we will show a dialog to let them associate the edit and the event; see pictures in task description of T400953. Then, in the event details page, we will show a list of all edits associated with that event, both individually and in aggregated form; see wireframes in T402211.

Table definition

CREATE TABLE ce_event_contributions (
  cec_id BIGINT UNSIGNED AUTO_INCREMENT NOT NULL,
  cec_event_id BIGINT UNSIGNED NOT NULL,
  cec_user_id BIGINT UNSIGNED NOT NULL,
  cec_wiki VARCHAR(64) NOT NULL,
  cec_page_id INT UNSIGNED NOT NULL,
  cec_page_prefixedtext VARBINARY(512) NOT NULL,
  cec_revision_id BIGINT UNSIGNED NOT NULL,
  cec_edit_flags INT NOT NULL,
  cec_bytes_delta INT NOT NULL,
  cec_links_delta SMALLINT NOT NULL,
  cec_timestamp BINARY(14) NOT NULL,
  cec_deleted TINYINT(1) NOT NULL,

  INDEX cec_wiki_page_id (cec_wiki, cec_page_id),
  INDEX cec_event_user (cec_event_id, cec_user_id),

  PRIMARY KEY(cec_id)
);

Table information

From wikitech

Should this table be replicated to wiki replicas (does it not contain private data)?

Data is partly public, partly private. More specifically, public replicas should only include rows such that: joining the ce_participants table on equal event+user yields a row where cep_private is false, and cep_unregistered_at is null. But, because this is in x1 and x1 tables are not publicly replicated, maybe we don't need any of that.

Will you be doing cross-joins with the wiki metadata?

Not with core tables, because we're in a different DB (cluster) anyway. However, some of the data that we store references core tables; for example, we have a column that references revision.rev_id. These are not foreign keys at the DB level though, also because they reference data from multiple wikis.

Size of the table (number of rows expected)

Well, initially 0. Then it'll grow as described below.

Expected growth per year (number of rows)

We aren't sure because there's currently no accurate tracking of this data anywhere (the P&E Dashboard would have higher estimates). As a starting point, we could probably assume an average of 1000 edits per event; given our target of 1800 events per year, that'd give 1.8M new rows per year as a generous estimate.

Expected amount of queries, both writes and reads (per minute, per hour...per day, any of those are ok)
  • Writes: One write per associated edit, and no other writes. So, using the estimate above, 1.8M writes/year or about 5k writes/day.
  • Reads: Only when someone goes to the metrics page. I don't think we have estimates for this, but if I were to guess, I'd put it in the order of magnitude of 10^4 per year.
Examples of queries that will be using the table.
Summary, organizer
SELECT
  SUM( IF (cec_bytes_delta > 0, cec_bytes_delta, 0 ) ) as positive_bytes,
  SUM( IF (cec_bytes_delta < 0, cec_bytes_delta, 0 ) ) as negative_bytes,
  SUM( IF (cec_links_delta > 0, cec_links_delta, 0 ) ) as positive_links,
  SUM( IF (cec_links_delta < 0, cec_links_delta, 0 ) ) as negative_links,
  COUNT( DISTINCT cec_user_id ) as participants,
  COUNT( DISTINCT cec_wiki ) as wikis,
  COUNT( DISTINCT CONCAT( cec_wiki, '|', cec_page_prefixedtext ) ) as pages,
  SUM( IF (cec_edit_flags & 1, 1, 0 ) ) as creations
FROM ce_event_contributions
JOIN ce_participants ON ( cep_event_id=cec_event_id AND cep_user_id=cec_user_id AND cep_unregistered_at IS NULL )
WHERE cec_event_id = 123 AND cec_deleted = 0
Table default, organizer
SELECT * FROM ce_event_contributions JOIN ce_participants ON ( cep_event_id=cec_event_id AND cep_user_id=cec_user_id AND cep_unregistered_at IS NULL ) WHERE cec_event_id = 123 AND cec_deleted = 0 ORDER BY cec_timestamp ASC, cec_id ASC LIMIT 50
Table sort 1, participant
SELECT * FROM ce_event_contributions JOIN ce_participants ON ( cep_event_id=cec_event_id AND cep_user_id=cec_user_id AND cep_unregistered_at IS NULL AND (cep_user_id = 333 OR cep_private = 0) ) WHERE cec_event_id = 123 AND cec_deleted = 0 AND ( cec_bytes_delta < 42 OR (cec_bytes_delta = 42 AND ( cec_timestamp < '456' OR ( cec_timestamp = '456' AND cec_id < 789 ) )) ) ORDER BY cec_bytes_delta DESC, cec_timestamp DESC, cec_id DESC LIMIT 500
Table sort 2, organizer
SELECT * FROM ce_event_contributions JOIN ce_participants ON ( cep_event_id=cec_event_id AND cep_user_id=cec_user_id AND cep_unregistered_at IS NULL ) WHERE cec_event_id = 123 AND cec_deleted = 0 AND ( cec_wiki > 'xywiki' OR ( cec_wiki = 'xywiki' AND ( cec_timestamp > '456' OR ( cec_timestamp = '456' AND cec_id > 789 ) ) ) ) ORDER BY cec_wiki ASC, cec_timestamp ASC, cec_id ASC LIMIT 100
Update record on page move
UPDATE ce_event_contributions SET cec_page_prefixedtext = 'Foo' WHERE cec_wiki = 'awiki' AND cec_page_id = 1234
Update record on page deletion
UPDATE ce_event_contributions SET cec_deleted = 1 WHERE cec_wiki = 'awiki' AND cec_page_id = 1234
The release plan for the feature (are there specific wikis you'd like to test first etc)

Features related to this table will be behind a feature flag. We'll test it in beta first, and then on to production wikis.

Open questions (obsolete)

What recommendations would DBAs have on indexes? As can be seen from above, all queries will filter on cec_event_id, then do a join on cec_event_id and cec_user_id, then allow pagination/sorting on the following unique tuples:

  • cec_page_prefixedtext + cec_wiki + cec_timestamp + cec_id
  • cec_wiki + cec_timestamp + cec_id
  • cec_user_id + cec_timestamp + cec_id
  • cec_timestamp + cec_id
  • cec_bytes_delta + cec_timestamp + cec_id

The current implementation has an index on (cec_event_id, cec_user_id) to cover the base filtering, but this won't do for pagination on other fields. I'm not sure what would help though. I did some quick tests but it would still filesort, e.g. with an index on (cec_event_id, cec_user_id, cec_timestamp, cec_id) in the "Table default, organizer" example above.

Event Timeline

Daimona added a project: DBA.

Hi DBAs! This task is ready for your review, and also advice given the open question about indices. Thank you!

Daimona renamed this task from [WIP] DBA review: new table for associating edits and events to DBA review: new table for associating edits and events.Aug 22 2025, 10:02 PM

Hi, We are a bit understaffed and dealing with aftermath an incident. Can it wait for a day or two?

Hi, We are a bit understaffed and dealing with aftermath an incident. Can it wait for a day or two?

Sure, thank you!

From the infrastructure point of view, since this is x1 table, I don't think we have any issues. I have a couple of comments on the data modelling:

cec_page_prefixedtext

pages get renamed (or namespaces) and this can become a mess. Are you sure you want to hold this? If events don't hold a lot of edits, you can just parse it from page table? Do multiple queries.

cec_edit_flags

What does this do? you mean tags?

cec_bytes_delta

BIGINT is a bit excessive here? Max page size is 2MB. Are you counting file upload size?

cec_links_delta

BIGINT is definitely excessive here. How many links are you planning to add that 4B is not enough? :D

On the question of indexes: As long as you have index on event_id, I don't think you need anything more. filesort is fine as long as the number of rows being scanned is small, if the key is picked and total number of rows is let's say 4K rows, it's still nothing. The discussion changes when you might end up with events with millions of edits (Maybe for now we can put a limit on number of edits associated with an event?)

cec_page_prefixedtext

pages get renamed (or namespaces) and this can become a mess. Are you sure you want to hold this? If events don't hold a lot of edits, you can just parse it from page table? Do multiple queries.

I put a bit of an explanation about this in the schema comments. Basically the issue is that these pages could be on a different wiki. Storing namespace ID + title dbkey is not an option because then we wouldn't be able to get the namespace name cross-wiki (T226667). The alternative would be storing the page ID, which I suppose is what you suggested. However, that still means we need to query the page table on multiple wikis. Also, I'm not sure how well cross-wikiness is supported by the relevant abstractions. Maybe PageStore works fine, but for example LinkBatch/LinkCache wouldn't work (T393653).

So, because we basically only need the titles so we can link them, I thought storing the prefixedtext was the simplest way out, as we could just feed that to WikiMap::getForeignURL. That being said, this is indeed quite crappy, so I'm very much open to alternatives.

cec_edit_flags

What does this do? you mean tags?

It's a bit field with additional properties of the edit. In practice, we'd currently only put 1 for page creations (so they can be easily filtered out). It may or may not be expanded in future with other things (like minor edits).

cec_bytes_delta

BIGINT is a bit excessive here? Max page size is 2MB. Are you counting file upload size?

Whooops. I completely missed reviewing the datatypes before making this task. We can make this an UNSIGNED INTEGER, same as revision.rev_len.

cec_links_delta

BIGINT is definitely excessive here. How many links are you planning to add that 4B is not enough? :D

Lol. I don't think there's a reference size here because this number is never stored anywhere AFAICT, but I suppose a smallint (2B) would be more than enough to cover even extreme pathological cases that probably don't even exist.

On the question of indexes: As long as you have index on event_id, I don't think you need anything more. filesort is fine as long as the number of rows being scanned is small, if the key is picked and total number of rows is let's say 4K rows, it's still nothing. The discussion changes when you might end up with events with millions of edits (Maybe for now we can put a limit on number of edits associated with an event?)

All these queries will come from a pager class, so they're guaranteed to have a LIMIT X with X <= 5000, even if somehow an event ends up with millions of edits (there's no hard limit in place). So yeah, filesort-wise we should be OK. Still though, maybe we need the same indices for pagination? For each tuple in the task description, we're going to have offset queries for pagination, like (cec_user_id, cec_timestamp, cec_id) > ('Admin', '20250101120000', 1234). I think these would be able to use an extended index, e.g. (cec_event_id, cec_user_id, cec_timestamp) (we can probably leave out the cec_id, it's only there for disambiguation in the event of rows with the same timestamp).

cec_page_prefixedtext

pages get renamed (or namespaces) and this can become a mess. Are you sure you want to hold this? If events don't hold a lot of edits, you can just parse it from page table? Do multiple queries.

I put a bit of an explanation about this in the schema comments. Basically the issue is that these pages could be on a different wiki. Storing namespace ID + title dbkey is not an option because then we wouldn't be able to get the namespace name cross-wiki (T226667). The alternative would be storing the page ID, which I suppose is what you suggested. However, that still means we need to query the page table on multiple wikis. Also, I'm not sure how well cross-wikiness is supported by the relevant abstractions. Maybe PageStore works fine, but for example LinkBatch/LinkCache wouldn't work (T393653).

ACK. Just saying this is going to be a mess to maintain, pages and namespaces rename constantly.

So, because we basically only need the titles so we can link them, I thought storing the prefixedtext was the simplest way out, as we could just feed that to WikiMap::getForeignURL. That being said, this is indeed quite crappy, so I'm very much open to alternatives.

cec_edit_flags

What does this do? you mean tags?

It's a bit field with additional properties of the edit. In practice, we'd currently only put 1 for page creations (so they can be easily filtered out). It may or may not be expanded in future with other things (like minor edits).

ACK

cec_bytes_delta

BIGINT is a bit excessive here? Max page size is 2MB. Are you counting file upload size?

Whooops. I completely missed reviewing the datatypes before making this task. We can make this an UNSIGNED INTEGER, same as revision.rev_len.

delta can go negative too, I suggest SIGNED INTEGER which still should be more than enough.

cec_links_delta

BIGINT is definitely excessive here. How many links are you planning to add that 4B is not enough? :D

Lol. I don't think there's a reference size here because this number is never stored anywhere AFAICT, but I suppose a smallint (2B) would be more than enough to cover even extreme pathological cases that probably don't even exist.

smallint goes to 32K (unsigned to 65K) which still think is okay worst case go with medium int or int. But make sure to go with signed as links can be also removed and go negative too.

On the question of indexes: As long as you have index on event_id, I don't think you need anything more. filesort is fine as long as the number of rows being scanned is small, if the key is picked and total number of rows is let's say 4K rows, it's still nothing. The discussion changes when you might end up with events with millions of edits (Maybe for now we can put a limit on number of edits associated with an event?)

All these queries will come from a pager class, so they're guaranteed to have a LIMIT X with X <= 5000, even if somehow an event ends up with millions of edits (there's no hard limit in place). So yeah, filesort-wise we should be OK. Still though, maybe we need the same indices for pagination? For each tuple in the task description, we're going to have offset queries for pagination, like (cec_user_id, cec_timestamp, cec_id) > ('Admin', '20250101120000', 1234). I think these would be able to use an extended index, e.g. (cec_event_id, cec_user_id, cec_timestamp) (we can probably leave out the cec_id, it's only there for disambiguation in the event of rows with the same timestamp).

I suggest just adding an index one index on event_id, timestamp. The rest, it can scan and discard. Won't be too bad.

cec_page_prefixedtext

pages get renamed (or namespaces) and this can become a mess. Are you sure you want to hold this? If events don't hold a lot of edits, you can just parse it from page table? Do multiple queries.

I put a bit of an explanation about this in the schema comments. Basically the issue is that these pages could be on a different wiki. Storing namespace ID + title dbkey is not an option because then we wouldn't be able to get the namespace name cross-wiki (T226667). The alternative would be storing the page ID, which I suppose is what you suggested. However, that still means we need to query the page table on multiple wikis. Also, I'm not sure how well cross-wikiness is supported by the relevant abstractions. Maybe PageStore works fine, but for example LinkBatch/LinkCache wouldn't work (T393653).

ACK. Just saying this is going to be a mess to maintain, pages and namespaces rename constantly.

Yeah, I'm genuinely looking to see what would be the best way to address this. I also realized a serious oversight, namely not planning for deleted revisions, for which I made T403200. I'll think about page moves at the same time and update the schema accordingly.

delta can go negative too, I suggest SIGNED INTEGER which still should be more than enough.
[...]
But make sure to go with signed as links can be also removed and go negative too.

Yep, sorry. The unsigned are a leftover from a previous iteration of the schema. Also my fault for not carefully checking the data types when I took over the patch.

I suggest just adding an index one index on event_id, timestamp. The rest, it can scan and discard. Won't be too bad.

Makes sense, then that would be a few more indices for each of the things we can filter/sort/page by. So, all in all, the table would have the following indices:

  • (cec_event_id, cec_user_id, cec_page_prefixedtext, cec_wiki, cec_timestamp)
  • (cec_event_id, cec_user_id, cec_wiki, cec_timestamp)
  • (cec_event_id, cec_user_id, cec_timestamp)
  • (cec_event_id, cec_user_id, cec_bytes_delta, cec_timestamp)

I will think a bit more about the changes needed for page moves/deletions and post the updated schema for review at that point.

I've updated the table definition with new columns for T403200, and also fixed the integer data types as described above. Now I will need to:

  • Update example queries (use page ID instead of title where it makes sense; add cec_deleted check to all queries)
  • Add index for row updates in case of page move/deletion (probably on (cec_wiki, cec_page_id))
  • Add/adjust indexes as discussed above + for new columns

Okay, I have made all of the updates to columns and data types and to the example queries. I've also reviewed the indices and re-run some tests. The current proposal is to leave only the two indices currently present in the table definition above, cec_wiki_page_id and cec_event_user. Everything else we can let the DBMS scan and discard as mentioned above. The cec_deleted column in particular doesn't seem worth indexing, because deleted rows are going to be a minuscule minority almost every time. Any other indices can be added later on if need be.

@Ladsgroup This is ready for review again. All the previous suggestions have been incorporated in the table definition. Thanks!

Good to go! It has DBA sign off. Just make sure you catalog it. Thank you for flying with DBA airlines.

Daimona assigned this task to Ladsgroup.

Good to go! It has DBA sign off. Just make sure you catalog it. Thank you for flying with DBA airlines.

Thank you! I'll close this task then since the table creation and cataloguing is tracked in the parent task (T400719). (I'll inquire separately about frequent flyer rewards :P)