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

make smartMerger.Merge merge reports in parallel #1827

Merged
merged 1 commit into from
Aug 24, 2016
Merged

Conversation

rade
Copy link
Member

@rade rade commented Aug 23, 2016

for reduced latency

@rade rade force-pushed the parallel-merger branch from 0775542 to b4aa605 Compare August 23, 2016 23:35
@rade
Copy link
Member Author

rade commented Aug 23, 2016

I put a little bit of load onto our dev cluster by having five browser tabs looking at the "Weave Cloud (Prod)" instance, plus one looking at Dev. I waited for query latencies to stabilise, then deployed this branch, waited to stabilise, then re-deployed master. Here is the result:
screenshot_2016-08-24_00-24-53
The branch was deployed at 23:12, and master re-deployed at 23:19.

@rade rade force-pushed the parallel-merger branch from b4aa605 to 05d607b Compare August 23, 2016 23:45
// fine as reports with the same ID are assumed to be the same.
nodes := []*node{}
seen := map[uint64]struct{}{}
l := len(reports)

This comment was marked as abuse.

This comment was marked as abuse.

@rade rade force-pushed the parallel-merger branch from 05d607b to 902ba88 Compare August 24, 2016 07:02
@tomwilkie
Copy link
Contributor

Lgtm!

@rade rade merged commit e8ebb31 into master Aug 24, 2016
@2opremio
Copy link
Contributor

Sorry to comment on this late but while the new merger clearly is advantageous for the WeaveCloud use I would be interested to see whether it makes sense when running the app in standalone. @rade Did you get to measure it? I would be particularly interested to see how it behaves in systems with a reduced number of cores.

@rade
Copy link
Member Author

rade commented Aug 24, 2016

I didn't measure it standalone. With few cores there is some cost due to pre-emption, but I doubt it's high. Plus we benefit from the better handling of unbalanced input / intermediate result sizes.

@rade rade deleted the parallel-merger branch July 5, 2017 13:08
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.

3 participants