Skip to content

fix(cron): stop the endless option hash update failures - #5001

Draft
solracsf wants to merge 1 commit into
mainfrom
fix/option-hash-collision-cron
Draft

solracsf wants to merge 1 commit into
mainfrom
fix/option-hash-collision-cron

Conversation

@solracsf

@solracsf solracsf commented Sep 6, 2026 •

Copy link
Copy Markdown
Member

The problem

JanitorCron logs Error updating option hash for optionId {id} every night, forever.

Two option rows of the same poll can hold different stored hashes and still resolve to the same recalculated hash: after an option text or duration change that never rewrote the hash, or with the empty default hash of an old install. UNIQ_options (poll_id, poll_option_hash, timestamp) rejects the second row's update, updateOptionHashes() catches, logs and skips it, and nothing repairs the row, so the next run does the same. deleteDuplicates() never sees those pairs, because it compares the stored hashes and not the recalculated ones.

Worse than the log noise: updateVoteHashes() succeeds where updateOptionHashes() failed, so the votes move to the new hash while their option keeps the old one. In the same run purgeDeletedOptions() removes the deleted twin and removeOrphanedVotes(), which joins on poll_id + hash, deletes those votes. updateHashes() already warns "Do not catch any exceptions ... Otherwise data loss of votes can occur" - the per-row catches were doing exactly that. Reproduced on MariaDB and PostgreSQL.

The fix

  • Group options and votes by their recalculated key before the hashes are written and drop the redundant rows. The survivor is a live option over a deleted one, a confirmed one over an unconfirmed one, then the lowest id, so a poll's result is never the row that goes.
  • Count rows whose hash could not be written and skip the orphan sweep while that count is not zero, so a swallowed error cannot cascade into a delete.
  • Option::getTimestampInDB(), because the unique index keys on the stored timestamp column, not on getTimestamp(), which is derived from iso_timestamp.

Two Oracle-only bugs made the repair impossible there. '' is stored as null, so the nullable hash columns hydrated null into a protected string and threw a TypeError - an Error, not an Exception, so nothing caught it and the whole cron run died on exactly the legacy rows this repair targets. And VoteMapper::update() reloads through buildQuery(), which groups by the primary key while selecting all columns, which Oracle rejects with ORA-00979; the maintenance path has no use for the joined attributes, so updateHash() writes without the reload.

Tests

New tests/Unit/Db/TableManagerTest.php, 7 cases, red before and green after. Reverting any one of the three non-obvious decisions (getTimestampInDB(), the confirmed tie-break, the nullable hash property) makes exactly one of them fail.

Worth knowing when reviewing: occ app:enable polls does not create the unique indices, so occ polls:db:reset-unique-indices is needed for any test to exercise the constraint.

Affected instances are repaired by the next cron run; occ polls:db:rebuild fixes them now.

The content of this PR was fully reviewed using AI

@solracsf solracsf self-assigned this Sep 6, 2026
@solracsf solracsf added the bugfix label Sep 6, 2026
Two option rows of the same poll can hold different stored hashes and still
resolve to the same recalculated hash, i.e. after an option text or duration
change that never got the hash rewritten, or with the empty default hash of an
old install. UNIQ_options over poll_id, poll_option_hash and timestamp then
rejects the second row's hash update, TableManager logs "Error updating option
hash for optionId" and skips the row. Nothing repairs it, so JanitorCron
produces the same error every night.

deleteDuplicates() cannot see those pairs, because it compares the stored
hashes and not the recalculated ones.

The log noise is not the worst of it: updateVoteHashes() succeeds where
updateOptionHashes() failed, so the votes move to the new hash while their
option keeps the old one. JanitorCron then purges the deleted twin and runs
removeOrphanedVotes(), which joins on poll_id and hash, and deletes the votes.

Group options and votes by their recalculated key before the hashes are written
and drop the redundant rows, keeping a live option over a deleted one and a
confirmed one over an unconfirmed one, so a poll's result is never the row that
gets dropped. Count the rows that could not be written and let JanitorCron skip
the orphan sweep while that count is not zero, so a swallowed error cannot
cascade into a vote deletion again.

Two things that kept this from working on Oracle at all:

Oracle stores an empty string as null, so the nullable hash columns read back as
null and hydrating the entity threw a TypeError. That is an Error, not an
Exception, so nothing caught it and the whole cron run died on exactly the
legacy rows this repair is meant to fix.

VoteMapper::update() reloads the entity through buildQuery(), which groups by
the primary key while selecting all columns. Oracle rejects that with
ORA-00979, so a vote hash could never be rewritten. The maintenance path has no
use for the joined attributes, so write the hash without the reload.

Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com>
@solracsf
solracsf force-pushed the fix/option-hash-collision-cron branch from 4d223c1 to c1c3037 Compare September 6, 2026 21:06
@dartcafe

dartcafe commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for your try, but:

  1. The janitor's job is to tidy up, not to fix database inconsistencies.
  2. The situation you face should be fixed by a recent migration or by calling occ polls:db:recreate. It exists exactly for these kind of problems. i.e. because it explicitly removes the unique indices before the repair run.

Your solution may work, but you are facing an inconsistency, which should be fixed once by recreating the database structure. After that this combination will never appear again, prevented by the unique index. Therefore a regularly fix job is not necessary.

It is always a good idea to add an issue before adding a PR.

So please make sure, that the issue is solvable by occ. Otherwise explain your issue first as a new bug report, so that the problem can be analyzed.

@dartcafe
dartcafe marked this pull request as draft September 29, 2026 17:21

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants