Skip to content

Added atomic newsletter email counters with drift comparison - #30674

Draft
jonatansberg wants to merge 3 commits into
jonatan-ber-3933-transactional-email-eventsfrom
jonatan-ber-3934-email-counters
Draft

Added atomic newsletter email counters with drift comparison#30674
jonatansberg wants to merge 3 commits into
jonatan-ber-3933-transactional-email-eventsfrom
jonatan-ber-3934-email-counters

Conversation

@jonatansberg

@jonatansberg jonatansberg commented Sep 10, 2026

Copy link
Copy Markdown
Member

Newsletter analytics currently recount recipient history to refresh email statistics. The transactional flush in #30668 now tells us exactly which recipients changed, so we can maintain delivered, opened and failed counts in that same transaction. Replaying a delivery then leaves its count at one, even after a lost commit acknowledgement.

This is the email-only counter step. It lets us validate counters independently of the member preparation work in #30671 and its dependencies. Existing email counter columns are sufficient; there is no migration or historical email backfill.

flush one newsletter email
  begin transaction
    lock email row
    first touch after startup → recount existing facts, including opens
    lock eligible recipient rows in primary-key order
    write timestamps + increment counters from exact transitions
  commit
  remember that this worker established the baseline

aggregate
  lock email row → recount all three outcomes → report differences
  recount member statistics, including replayed members

The startup recount corrects existing counters once per touched email per worker lifetime. It normally covers active sends and the fetch trust window; scheduled recovery can also touch an older email. Baseline and increments share the email lock, so concurrent workers and retries can safely repeat initialization. Subsequent comparisons observe drift without repairing it. They always include opens, even when the fetch lane passes includeOpenedEvents: false.

The new emailAnalytics.emailCounterMode defaults to off. compare requires batchProcessing: true; mode changes require draining existing analytics workers and restarting with consistent configuration. Legacy writers do not take the counter lock. Member recomputation and its replay recovery remain in place. Automation/gift persistence and opens-first scheduling are unchanged.

Comparison logs signed differences and records comparison counts plus absolute observed drift by event type. See the email analytics README for startup, rollback and metric semantics. A restart rebaselines touched emails, so operators should inspect drift before restarting.

Validation:

  • 108 MySQL email-service integration tests and 184 analytics unit tests passed.
  • The final focused counter suite passed all 7 tests: existing opens at cutover, missing-lane comparison, deliberately wrong counters surviving comparison/replay, transaction rollback, lost commit acknowledgement plus restart, concurrent baselines, and comparison overlapping an uncommitted event transaction.
  • Core/test TypeScript checks, focused lint and commit hooks passed. pnpm check stops on the same 452 pre-existing formatting files; none is changed here.
  • Five independent slice reviews and five accumulated-stack review passes found no high-confidence issues.
  • Synthetic benchmark: 5,000 members, 500 historical emails, 2,009,528 recipient rows, 5,000 events per cycle, 0/50/100% duplicate opens. All email/member truth assertions passed. Median handler reads increased 0.0023–0.0028% relative to Changed batched newsletter events to use exact transactional transitions #30668, including startup baselining. Comparison retains recount load; this does not claim the later read reduction. The harness uses innodb_flush_log_at_trx_commit=0 and no binlog, so absolute write timings are not production estimates.

Based on #30668; independent of the schema-only #30671. Removing mid-cycle email recounts follows after comparison has established correctness.

ref https://linear.app/ghost/issue/BER-3934
ref https://linear.app/ghost/issue/BER-3937

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Comment @coderabbitai help to get the list of available commands.

@nx-cloud

nx-cloud Bot commented Sep 10, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 5ce34bd

Command Status Duration Result
nx run ghost:test:integration ✅ Succeeded 3m 32s View ↗
nx run ghost:test:legacy ✅ Succeeded 3m 17s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 48s View ↗
nx run ghost:test:ci:integration ✅ Succeeded 1s View ↗
nx run ghost:test:ci:e2e ✅ Succeeded 3s View ↗
nx run-many -t test:types -p ghost ✅ Succeeded 7s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded <1s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 2s View ↗
Additional runs (4) ✅ Succeeded ... View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-09-11 14:50:21 UTC

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.38739% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.78%. Comparing base (b3e2c0d) to head (5ce34bd).

Files with missing lines Patch % Lines
...vices/email-analytics/newsletter-email-counters.ts 87.67% 1 Missing and 8 partials ⚠️
...core/core/server/services/email-analytics/index.ts 68.75% 1 Missing and 4 partials ⚠️
Additional details and impacted files
@@                               Coverage Diff                               @@
##           jonatan-ber-3933-transactional-email-events   #30674      +/-   ##
===============================================================================
+ Coverage                                        67.75%   67.78%   +0.02%     
===============================================================================
  Files                                             1670     1672       +2     
  Lines                                            60349    60448      +99     
  Branches                                         10436    10454      +18     
===============================================================================
+ Hits                                             40891    40975      +84     
- Misses                                           17151    17153       +2     
- Partials                                          2307     2320      +13     
Flag Coverage Δ
e2e-tests 70.55% <87.38%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ref https://linear.app/ghost/issue/BER-3934/add-incremental-email-and-member-analytics-counters

Establish an opens-inclusive baseline per touched email at startup and commit
exact event increments with recipient transitions. Compare derived truth
without repairing established counters so rollout can expose real drift.
Keep member recounts and replay recovery while preparation accounting lands.
ref https://linear.app/ghost/issue/BER-3934/add-incremental-email-and-member-analytics-counters
ref https://linear.app/ghost/issue/BER-3937

Review found that comparison mode could never observe drift on an
email's first touch after a restart: the baseline replaced the counters
with truth before the comparison read them, so a wrong counter left by
an earlier process always compared as zero drift. The baseline now logs
and records the correction it applies as a separate metric, and a
comparison reuses that truth instead of recounting the same email twice
inside one locked transaction.

The recount also returns to one covering-index count per outcome, as the
legacy recount did, because a single three-column count reads every
recipient row while the email row is locked; comparison reads only the
counter columns instead of the whole email. Comparison transactions now
retry lock waits and deadlocks through the same helper the event flush
uses, so one lock wait no longer aborts member aggregation for the whole
cycle. A missing email row is no longer remembered as initialized, and
an unknown or unsupported counter mode is logged at boot instead of
silently behaving like off.
ref https://linear.app/ghost/issue/BER-3934/add-incremental-email-and-member-analytics-counters

New test files follow the repository's TypeScript direction, which the
CodeRabbit "New files are TypeScript" check enforces for this PR.
@jonatansberg
jonatansberg force-pushed the jonatan-ber-3934-email-counters branch 2 times, most recently from e21be7d to 5ce34bd Compare September 11, 2026 14:41
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.

1 participant