NONEVM-3038: logpoller global filter cache#585
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces an in-memory filter cache to the logpoller service to eliminate redundant database queries during polling operations. The cache is lazily loaded from the database on first access and kept synchronized through RegisterFilter and UnregisterFilter operations.
Changes:
- Added
GetAllActiveFiltersmethod toFilterStoreinterface with postgres, in-memory, and observed implementations - Implemented lazy-loading filter cache in
servicewith thread-safe access viafiltersMu - Modified
buildFilterIndexandgetDistinctAddressesto read from cache instead of querying the database - Updated
RegisterFilterto short-circuit on cache hit when filter configuration is unchanged
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/logpoller/store/postgres/filters.go | Added GetAllActiveFilters to retrieve all non-deleted filters from postgres |
| pkg/logpoller/store/memory/filters.go | Implemented GetAllActiveFilters for in-memory filter store |
| pkg/logpoller/service.go | Added filter cache fields, loadFilters helper, and updated processBlockRange signature |
| pkg/logpoller/observed_filter_store.go | Added metrics wrapper for GetAllActiveFilters |
| pkg/logpoller/interfaces.go | Extended FilterStore interface with GetAllActiveFilters method |
| pkg/logpoller/filter_test.go | Added comprehensive test coverage for filter cache behavior |
| pkg/logpoller/filter.go | Refactored filter operations to use cache with lazy loading |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f := filters[i] | ||
| lp.filtersByName[f.Name] = &f |
There was a problem hiding this comment.
Don't need to introduce this var
| f := filters[i] | |
| lp.filtersByName[f.Name] = &f | |
| lp.filtersByName[filters[i].Name] = &filters[i] |
| if cached, ok := lp.filtersByName[filter.Name]; ok { | ||
| // Filter exists - check if config matches | ||
| if cached.Address.String() == filter.Address.String() && | ||
| cached.MsgType == filter.MsgType && |
There was a problem hiding this comment.
Just double check here, the cache check only compares Address/MsgType/EventSig, so another filter with different LogRetention, MaxLogsKept, or StartingSeqNo would be silently skipped. That's intentional right?
Summary
filtersByName) to eliminate redundant DB queries on every poll tickloadFilters(), then kept in sync byRegisterFilter/UnregisterFilterbuildFilterIndexandgetDistinctAddressesnow read from cache instead of hitting DBHasFilterreturns from cache with no DB round-tripRegisterFiltershort-circuits on cache hit when filter config is unchangedGetAllActiveFilterstoFilterStoreinterface with postgres, in-memory, and observed implementations