Skip to content

Add cached impression writer for batching database writes#1168

Draft
ericholscher wants to merge 3 commits intomainfrom
claude/cached-impression-writes-OGDPk
Draft

Add cached impression writer for batching database writes#1168
ericholscher wants to merge 3 commits intomainfrom
claude/cached-impression-writes-OGDPk

Conversation

@ericholscher
Copy link
Copy Markdown
Member

Summary

Implement a caching layer for ad impression tracking to reduce database load by batching writes. Instead of writing to the database on every impression event (offer, view, click), impressions are now accumulated in Django's cache and periodically flushed to the database in batch.

Key Changes

  • New CachedImpressionWriter class (impression_cache.py): Buffers impression increments in Django's cache with deterministic cache keys based on ad, publisher, date, and impression type. Provides increment() to add to cached counters and flush() to batch-write to the database.

  • Updated Advertisement.incr() method (models.py): Added conditional logic to use the cached writer when ADSERVER_IMPRESSION_CACHE_ENABLED is True, falling back to direct database writes when disabled.

  • New periodic task (tasks.py): Added flush_impression_cache() Celery task to periodically flush cached impressions to the database.

  • Configuration setting (settings/base.py): Added ADSERVER_IMPRESSION_CACHE_ENABLED environment variable (defaults to False) to control whether impression caching is active.

  • Comprehensive test suite (test_cached_impressions.py): 20 tests covering cache key generation, increment accumulation, dirty key tracking, database flushing, handling of existing records, multiple ads/impression types, null advertisements, concurrent safety, and integration with the Advertisement.incr() method.

Implementation Details

  • Cache keys use a deterministic format: impression_cache:{ad_id}:{publisher_id}:{date}:{impression_type}
  • Dirty keys are tracked in a separate cache entry to know which keys need flushing
  • Flush operation uses atomic F() expressions for safe concurrent updates to existing records
  • Supports null advertisement IDs for decisions with no associated offer
  • Cache timeout is set to 2 hours to provide a generous buffer for flush intervals
  • Graceful error handling with logging if individual flush operations fail

https://claude.ai/code/session_01Cpd47jJhFJ9MEkfobuzKSj

Buffer impression increments (views, clicks, offers, decisions) in Django's
cache instead of writing directly to the database on every ad request. A
periodic Celery task flushes the accumulated counts to the AdImpression table
in batch, reducing database write pressure.

- Add CachedImpressionWriter in adserver/impression_cache.py
- Gate the feature behind ADSERVER_IMPRESSION_CACHE_ENABLED setting (default off)
- Modify Advertisement.incr() to use the cache when enabled
- Add flush_impression_cache Celery task
- Add 16 tests covering cache operations, flush behavior, and integration

https://claude.ai/code/session_01Cpd47jJhFJ9MEkfobuzKSj
claude and others added 2 commits March 6, 2026 23:50
- Add flush_impression_cache to CELERY_BEAT_SCHEDULE in both production
  (every minute) and development settings so the task actually runs.
- Fix race condition where increments arriving during a flush window were
  lost: use cache.decr() to subtract only the snapshotted amount instead
  of deleting keys outright, preserving any concurrent increments.
- Fix race condition in dirty key tracking: add per-counter marker keys
  so concurrent _add_dirty_key calls don't clobber each other via
  read-modify-write on the shared set.
- Re-read the dirty key set before pruning fully-flushed keys so new
  entries added during the flush are not discarded.
- Add test_increments_during_flush_are_preserved to verify concurrent
  increment safety using mock-injected increments during cache.decr().

https://claude.ai/code/session_01Cpd47jJhFJ9MEkfobuzKSj
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.

2 participants