Page MenuHomePhabricator

Allow multiple tallies to be associated with a poll
Open, In Progress, Needs TriagePublic

Description

This is a prerequisite for T309416: SecurePoll: Allow re-tallying of STV elections with a pre-eliminated candidate. SecurePoll assumes that a poll will ever generate 1 tally so tallies are keyed to a poll and assumes that this association will be unique. For instance, the TallyElectionJob class uses the election id to confirm it should run and then writes a property, tally-result associated with that election id. T309416 will want different tallies to be associated with a single poll so all of these assumptions will have to be unwound. The tally page is keyed to an election id and assumes that one row in the db is associated with the tally it cares about and decides what to display based on that assumption.

Acceptance Criteria:

  • the tally page of an election allows a re-tally of an existing tally or a new tally to be generated
    • /tally/{electionId} should probably only show a table of existing tallies (with options to re/tally) and some new url schema, /tallyresults/{tallyId} should lead to the actual results. No strong opinion on what this implementation actually is. Can possibly pull in design if we have a lot of questions.
    • it should continue to show the status of each tally
  • multiple tallies can be associated with a single election
  • tallies should be deletable

As no modifications can be made to election parameters yet, the only thing this patch should do is allow someone to spam the same results across multiple tally results displays.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

I'm thinking it would make sense to move tallies into their own one to many table (election -> tallies) rather than keeping the results in securepoll_properties. We should be able to migrate all existing tallies into this new table.

Change #1137838 had a related patch set uploaded (by Amdrel; author: Amdrel):

[mediawiki/extensions/SecurePoll@master] Allow multiple tallies to be associated with a poll

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

Change #1138121 had a related patch set uploaded (by Amdrel; author: Amdrel):

[mediawiki/extensions/SecurePoll@master] Add 'migrateTallies' maintenance script

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

Change #1138122 had a related patch set uploaded (by Amdrel; author: Amdrel):

[mediawiki/extensions/SecurePoll@master] Allow multiple tallies to be associated with a poll

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

Amdrel changed the task status from Open to In Progress.Apr 24 2025, 12:06 AM
Dreamy_Jazz subscribed.

Have we asked for DBA input on this new table?

Have we asked for DBA input on this new table?

We haven't asked a DBA for input yet.

Number of securepoll tables is impacting performance of s3: See T355594#10878890 I prefer to either move these tables wholesale to x1 or at least get rid of some other tables before adding more.

If we resolved T355594 would it be okay to add this table? Otherwise @Amdrel could we continue using the securepoll_properties for this task? I looked over the securepoll tables that exist (ignoring the voter eligibility spam ones) and they're kind of a hodgepodge. Half contain multiple data types and half are single purpose storage tables.

Actually given the discussion we're having and the problems SecurePoll's table count is causing, on second thought maybe we should aim to not add this table. Am I correct in assuming that the table is being added because the pr_key is part of the primary key and it needs to be unique? If so, could we maybe make tally-result an array of results?

If we're certain that adding another table will cause issues (albeit one that should be small) then I can explore using the properties table.

  1. It should be possible to make tally-result an array of results, but I have to make sure that won't cause issues with the new pager as I believe it only works when directly interacting with a table and its columns rather than JSON.
  2. The pr_value column in securepoll_properties is limited to 16 MB in size. Uncompressed tally results in our test data are ~300K in size on average. This should allow us to fit plenty of tally results in pr_value as it's unlikely there would be more than a few per election.

I looked into how the pagers work a bit more and it looks like we would have to render a table directly with all tallies instead of using a pager if we move everything to securepoll_properties.

we would have to render a table directly with all tallies instead of using a pager if we move everything to securepoll_properties.

Yes I think this is true and is probably the easier technical lift, given that SecurePoll doesn't use the pager system to render a tally right now. I don't think we necessarily have to render all results though. I think an added URL parameter like tally/{electionId}/result/{tallyId} would allow us to render conditional tallies, assuming tallies are stored like:

[
  1 => [ // tally id - just has to be unique to the election, not unique across all elections
    params => // assuming this is supported
    results => // original result value

And if a user visits tally/{electionId}, it would make sense to render the list of tallies there.

Change #1137838 abandoned by Amdrel:

[mediawiki/extensions/SecurePoll@master] Created new 'securepoll_tallies' table for tallies

Reason:

Abandoning as this patch is no longer needed. The other related patches will remain open though.

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

Change #1138121 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Add 'migrateTallies' maintenance script

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

Change #1138122 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Allow multiple tallies to be associated with a poll

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

My localhost broke when visiting the tally page.

image.png (1×2 px, 181 KB)

I see a maintenance script above to do a migration.

Do we have a plan in place to run this maintenance script on all Wikimedia wikis that host non-redirect polls? I would assume this maintenance script needs to be run on the following wikis:

  • votewiki
  • testwiki
  • enwiki
  • officewiki
  • others?

https://gerrit.wikimedia.org/g/operations/mediawiki-config/+/b2424505fba681c5b0b38d8ac085421322b9578c/wmf-config/InitialiseSettings.php#10851

I assume if this maintenance script is not run before Tue/Wed/Thu, then as this patch rides the train, it will break the tally page on various wikis.

Do we have any third party users of SecurePoll? Do we need to think of a way to make this migration easier for them? For example, having the migration script run when they do php maintenance/run.php update?

I had trouble running the maintenance script at first, so I filed T398268: rename "cli" directory to "maintenance".

Change #1165561 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/SecurePoll@master] Don't fail on the tally list page if an older tally format is found

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

For some reason, I assumed we ran all of these automagically with whatever setup we had already. Given that this isn't the case and that I do think we might have third party users, I've pushed a patch to allow this to fail a little more gracefully. Depending on how critical access to tallies is, we might want to backport this? (even if it's just a temporary fix)

I just merged https://gerrit.wikimedia.org/r/1165561, which makes it so that SecurePoll just silently ignores tallies using the old format, instead of throwing an Exception. This makes running the maintenance script less important now. Perhaps we can skip running the maintenance script unless someone asks for it, or run it only for votewiki.

Why isn't running the maintenance script very important now? For newer polls, it should be easy enough for the election admin to re-tally them. For older polls, it would be hard for the election admin to re-tally them because they'd have to look up old encryption keys, however I think it's very rare to need to re-tally an old poll. The results are usually copy pasted onwiki somewhere for easy lookup, and there are no new votes or strikes, so those results do not change.

Change #1165561 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] Don't fail on the tally list page if an older tally format is found

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