Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces an in-memory cache system for the events processor to improve performance by reducing database queries. The cache uses BadgerDB for storage and implements Change Data Capture (CDC) via Debezium to keep cached data synchronized with the database in real-time.
Key Changes
- Implemented a BadgerDB-based in-memory cache with snapshot loading and CDC consumption
- Added utility types (
NullTime,StringArray) for JSON serialization/deserialization and database operations - Integrated cache usage in event enrichment services to replace direct database lookups
- Updated dependency versions in go.mod/go.sum
Reviewed changes
Copilot reviewed 45 out of 46 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| events-processor/cache/*.go | Core cache implementation with model-specific operations and CDC consumers |
| events-processor/models/*.go | Added streaming query utilities and updated models with new JSON tags |
| events-processor/utils/*.go | New utility types for nullable time and string arrays |
| events-processor/main.go | Cache initialization and lifecycle management |
| events-processor/processors/*.go | Integration of cache into event processing pipeline |
| extra/debezium_config.json | Debezium CDC configuration for Kafka topics |
| events-processor/go.mod | Updated dependencies including BadgerDB and OpenTelemetry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
0d32942 to
d3b2b19
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
events-processor/models/query_streaming.go:1
- Debug print statement left in production code. This should be removed or replaced with proper logging using the logger instance.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
groyoh
left a comment
There was a problem hiding this comment.
Be careful, you updated the submodules.
There was a problem hiding this comment.
Review done by Claude Code
Good work overall on this cache layer — the generic ConsumerConfig[T] / LoadSnapshot[T] patterns, the TTL-based soft delete for subscriptions, and the feature-flag gating are all solid choices.
A few issues below that I think should be addressed before merging, primarily around correctness differences between the cache and DB paths.
🟡 Removed test coverage for the reprocess scenario (processor_test.go)
The "When reprocess flag is set, only produces to enriched expanded topic" test was removed and not re-added in the new structure. This loses coverage for the reprocess functionality (checking that enrichedProducer, inAdvanceProducer, and cacheStore are NOT called, and only enrichedExpandedProducer is).
No description provided.