- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 432
 
Poll Memoizer #704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Poll Memoizer #704
Conversation
| @adapter = if memoize == :poll | ||
| Adapters::Poll.new(Adapters::Memory.new, adapter, | ||
| key: 'memoizer', | ||
| interval: 5, | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FIXME: make configurable
| @adapter = adapter | ||
| @adapter = if memoize == :poll | ||
| Adapters::Poll.new(Adapters::Memory.new, adapter, | ||
| key: 'memoizer', | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If multiple DSL instances are created with memoize: :poll and different adapters, this will not behave as expected. I wonder if Poller.get should also look for equality in the adapters. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the key just be unique per adapter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or rather a better way to phrase that, is there a way to make the key unique per instance? We have the name. That would help with different adapters. Perhaps there is another way to add uniqueness per instance too... hmm
| instrumenter: @instrumenter | ||
| ).tap do |poll| | ||
| # Force poller to sync in current thread now | ||
| poll.poller.sync | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be tempted to make a method for this to avoid reaching into poller.
| 
           I'm going to hold off on this for now since we're not using this adapter in production. Eventually I do think some kind of interval-based memoization (instead of request-based) will really help improve performance, but I'd like to think on it a bit.  | 
    
* origin/main: (454 commits) Use flippercloud.io API, don't retry failures Move rails constants Show deprecation warning for Rails < 6.1.0 Show deprecation warning for Ruby < 3.0 UI: show a badge if a new release is available Use GitHub Releases for changelog Fix warning from script/bootstrap Prepare for 1.1.2 release Update Changelog Fix remove_index for older Rails versions Add length to update migration Create instead of purge before all Indifferent access for old Rails versions Fix rescue that may be obscuring errors on Actions Skip AR tests locally if db can't connect Add rails to readme like summary Add feature flag to summaries Duplicate postgres env config Move postgres env config Configure postgres and mysql on CI ...
Currently, Flipper is configured by default to perform memoization and preloading per request, which will query the primary storage adapter each time a request is received. This is fine on a lot of apps, but can add some overhead on apps with heavy traffic.
This pull request adds a
memoize = :pollconfiguration option:Setting
memoize = :pollwill switch memoization to a new background thread polling strategy based on thePolladapter introduced in #682. ThePolladapter will start a background thread that will load all features from the primary storage adapter into aMemoryadapter on an interval. Flipper will then read from thisMemoryadapter instead of the primary storage adapter (it actually has multiple instances of theMemoryadapter, one for the background, and then one for each app thread that is synchronized before each request).TODO: