Skip to content

fix: omit None fields from click inserts to stop bucket churn - #332

Open
Zingzy wants to merge 1 commit into
mainfrom
fix/click-null-fields
Open

fix: omit None fields from click inserts to stop bucket churn#332
Zingzy wants to merge 1 commit into
mainfrom
fix/click-null-fields

Conversation

@Zingzy

@Zingzy Zingzy commented Aug 30, 2026

Copy link
Copy Markdown
Member

Problem

The clicks time-series collection averages 4.07 docs per bucket (66M docs across 16.2M buckets). The bucket index alone is 3.6GB, about half the database's total index footprint, and the resulting memory pressure is behind the recent Mongo incidents.

Root cause: click documents store empty analytics fields as explicit nulls (referrer: null, utm_source: null, bot_name: null). A time-series bucket cannot hold two BSON types in one column, so every alternation between a direct click (null referrer) and a referred click (string referrer) hard-closes the bucket with a schema-change rollover. Measured on prod: 1,035 of 10,254 inserts closed a bucket this way in one hour; the busiest link created 695 buckets in a single day from 1,106 null/string flips.

Fix

  • ClickDoc.to_mongo() now serialises with exclude_none=True, so empty fields are absent instead of null. Scoped to clicks only; the shared MongoBaseModel.to_mongo() is unchanged.
  • Bootstrap now creates the clicks collection with granularity: "hours", matching prod (collMod'd from "seconds" earlier). Fresh deploys previously got the churn-prone setting.

Why reads are safe

  • Group-bys map null and missing identically via $ifNull sentinels (Direct / unknown / (none)).
  • Sentinel filters already carry a {$exists: false} arm because clicks recorded before feat(analytics): add device and UTM click dimensions #259 lack the device/utm fields entirely; mixed shapes are already the norm in this collection.
  • Every nullable ClickDoc field has a default, so from_mongo handles absent keys.

Verification

  • Bucket behaviour on a local Percona Server for MongoDB 8.0: 200 alternating direct/referred clicks produce 200 buckets on main and 1 bucket with this change. Field-appears-later and int/double flips confirmed harmless; only null/type alternation closes buckets.
  • Mixed-shape equivalence through the real StatsService over a collection holding 120 old-shape and 120 new-shape docs: 15 exact-number checks (totals, every dimension group-by, sentinel filters including referrer=Direct, non-sentinel filters, per-link path) all match independently computed values.
  • Live app end to end, both sink modes: inline, and stream with the click worker consuming from Redis. Referred+utm, direct, and bot clicks via real HTTP; stored docs contain zero null values and only the keys with real values; public stats aggregate them correctly.
  • Full suite: 3,639 passed, 5 skipped. Lint and format clean.

Existing buckets keep their shape (time-series buckets are immutable in place); this stops new churn, and a history repack can reclaim the index separately.

Summary by CodeRabbit

  • Bug Fixes

    • Click data now omits optional fields when no value is provided, preventing unnecessary null values.
    • Empty metadata values are excluded from stored click records.
    • Click time-series storage now uses hourly granularity for improved alignment with production behavior.
  • Tests

    • Added coverage verifying omitted and populated click fields are serialized correctly.
    • Updated click-handler tests to reflect the streamlined stored data format.

Explicit nulls hard-close time-series buckets on every null-to-string
type flip (schemaIncompatible rollover): prod averages 4 clicks per
bucket across 16.2M buckets, and the bucket index alone is 3.6GB.
Verified on PSMDB 8.0: 200 alternating clicks made 200 buckets with
nulls present and 1 bucket with the fields omitted. Readers are
unaffected: group-bys use $ifNull and sentinel filters already carry
an $exists:false arm. Also aligns bootstrap granularity with prod
(hours, collMod'd from seconds) so fresh deploys bucket correctly.
Copilot AI lite review requested due to automatic review settings August 30, 2026 17:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 18d1b267-07f5-4a72-b033-a49dfd9cc21b

📥 Commits

Reviewing files that changed from the base of the PR and between 512d883 and 144ce43.

📒 Files selected for processing (5)
  • repositories/indexes.py
  • schemas/models/click.py
  • tests/unit/repositories/test_indexes.py
  • tests/unit/schemas/models/test_click.py
  • tests/unit/services/test_click_service.py

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The clicks time-series collection now uses hour granularity. ClickDoc.to_mongo() serializes aliased fields while excluding None values. Unit tests cover granularity, omitted fields, populated fields, UTM tags, and cached domains.

Changes

Click time-series alignment

Layer / File(s) Summary
Clicks time-series granularity
repositories/indexes.py, schemas/models/click.py, tests/unit/repositories/test_indexes.py
The clicks collection declaration and test now use "granularity": "hours".
ClickDoc omission-aware serialization
schemas/models/click.py, tests/unit/schemas/models/test_click.py, tests/unit/services/test_click_service.py
ClickDoc.to_mongo() excludes None fields. Tests verify omitted optional fields and preserved populated fields.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 144ce

The PR omits null-valued optional click fields and uses hourly bucketing for newly created collections while preserving required identity fields and existing collection behavior. No actionable merge-blocking risk remains beyond normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: omitting None fields from click inserts to reduce MongoDB time-series bucket churn.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/click-null-fields

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Zingzy Zingzy self-assigned this Aug 31, 2026
@Zingzy Zingzy added the backend Changes related to Backand/API label Aug 31, 2026
@Zingzy Zingzy moved this to 🏗️ In Progress in spoo.me Development Roadmap Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend Changes related to Backand/API

Projects

Status: 🏗️ In Progress

Development

Successfully merging this pull request may close these issues.

2 participants