Skip to content
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

HT-3026: Nightly etas overlap updates #97

Merged
merged 5 commits into from
Jul 16, 2021

Conversation

jsteverman
Copy link
Contributor

Caches cluster level counts. Could still be done for MPMs.

Caches cluster level counts. Could still be done for MPMs.
  Also ensures orgs w/holdings in a cluster and no matches get copy_counts
@jsteverman jsteverman marked this pull request as ready for review June 28, 2021 13:58
@jsteverman jsteverman force-pushed the HT-3026-nightly-etas-update branch from 0d715a7 to b4275f1 Compare June 29, 2021 17:57
@jsteverman jsteverman changed the title WIP: improved perf for Serial and SPM overlaps Improved perf for Serial and SPM etas overlaps Jul 12, 2021
@jsteverman jsteverman changed the title Improved perf for Serial and SPM etas overlaps HT-3026: Nightly etas overlap updates Jul 12, 2021
@jsteverman jsteverman requested review from aelkiss and mwarin July 13, 2021 14:06
@aelkiss
Copy link
Member

aelkiss commented Jul 13, 2021

Probably should be rebased, maybe should be squashed some.

Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I like that this moves some of the Mongoid-specific stuff out of the overlap computation; given that we have to fetch the entire cluster anyway, this seems like this minimizes the number of times we need to look through the entire set of holdings for a cluster.

It may be worth spending a little bit of time looking at performance vs. size of the cluster to make sure that there isn't still an n^2 issue. @mwarin and I set up a test case with synthetic data to do something similar with holdings updates, which was helpful to verify that our changes were actually making an improvement; it was also helpful for being able to run the profiler without getting extremely noisy data. I think both those things would be helpful to do here as well, but shouldn't hold up merging this.

@jsteverman jsteverman merged commit db0f79b into master Jul 16, 2021
@aelkiss aelkiss deleted the HT-3026-nightly-etas-update branch July 22, 2021 15:38
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