fetch reports in bulk from couch #34563
Conversation
| def fetch_reports(self, domain, report_configs): | ||
| """Bulk fetch all reports for syncing""" | ||
| report_ids = [config.report_id for config in report_configs] | ||
| self.reports = _get_reports_and_data_source(report_ids, domain) |
There was a problem hiding this comment.
There's a weird ordering dependency here around setting self.reports that would be nice to avoid if doable. Would it be possible to make report_configs and domain required parameters to __init__, and then make self.reports a cached_property? I think that'd mean moving the initialization from BaseReportFixtureProvider.__init__ to __call__.
There was a problem hiding this comment.
I added a few commits to update this:
- pass all report configs to cache from the start
- still call 'load' in the fixture provider to allow for error handling
- load reports just in time if needed (purely a safeguard)
- allow loading a subset of the reports to avoid loading unnecessary reports in the V2 only use case.
This only applies in the v1/v2 migration mode
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
|
That sentry error is real and it is new as of this PR, but it's essentially just a different manifestation. This scenario would previously raise an error like this: |
👍🏻 the error seems like it must be an artifact of linked domains |
Technical Summary
Fetch the report docs from couch in bulk instead of one at a time. In some DD traces I noticed that each fetch from couch was taking ~300ms which stacks up for multiple reports. The bulk fetch should be much faster than successive individual fetches.
One other change is to check a FF outside of the per-row loop. In practice this is a
locmemlookup but it still seems better to do it once instead of for every row.Easiest review by commit (last commit is only lint).
Safety Assurance
Safety story
Covered by existing unit tests.
Automated test coverage
Existing coverage
QA Plan
None
Rollback instructions
Labels & Review