Skip to content

fix(vote): serialize vote count refreshes - #1653

Open
lhk0504 wants to merge 1 commit into
apache:mainfrom
lhk0504:fix/vote-count-race
Open

lhk0504 wants to merge 1 commit into
apache:mainfrom
lhk0504:fix/vote-count-race

Conversation

@lhk0504

@lhk0504 lhk0504 commented Oct 22, 2026

Copy link
Copy Markdown

Fixes #1646

Proposed Changes

  • Acquire the question, answer, or comment's write lock before reading vote totals, then count and save within the same transaction. Concurrent refreshes for the same object are serialized, preventing a delayed request from overwriting a newer total.
  • Count active upvotes and downvotes in one grouped query and use that result for both the stored score and the returned counts. Propagate configuration/query errors and roll back failed refreshes. Resolve activity types before opening the transaction to avoid requiring a second SQLite connection; comments remain upvote-only.
  • Add deterministic two-connection regression tests for upvotes, cancellations, downvotes, and vote-direction changes, plus active-vote filtering, negative/zero totals, and error handling. The controlled stale-write regression fails against the original implementation and passes with this fix.

The lock is acquired with a no-op update that preserves the score and update timestamp. It is held through the count and final update, so refreshes on the same object wait for each other. SQLite uses its database write lock; PostgreSQL/MySQL use the target row's write lock.

Validation

  • go test ./internal/repo/activity ./internal/repo/repo_test ./internal/service/content -count=1
  • go test -race ./internal/repo/activity -count=1 -timeout 90s
  • go build ./...
  • go vet ./...
  • golangci-lint run ./internal/repo/activity/... (0 issues)
  • git diff --check

The committed tests use SQLite. The same regression tests also passed against a temporary local PostgreSQL 14.2 instance using a test-fixture adapter supplied through Go's -overlay option; that adapter is not part of this change. MySQL/MariaDB have not been tested.

Related: #1650 proposes another approach to the same issue.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concurrent vote updates may overwrite vote_count with stale totals

1 participant