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

1086 lb refactor #1087

Merged
merged 8 commits into from
Sep 25, 2020
Merged

1086 lb refactor #1087

merged 8 commits into from
Sep 25, 2020

Conversation

PhilMiller
Copy link
Member

Fixes #1086

Copy link
Collaborator

@lifflander lifflander 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

@PhilMiller
Copy link
Member Author

Failure of clang-8/macosx/mpich was seemingly due to #958

@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #1087 into develop will decrease coverage by 0.00%.
The diff coverage is 94.11%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1087      +/-   ##
===========================================
- Coverage    77.66%   77.65%   -0.01%     
===========================================
  Files          672      671       -1     
  Lines        25806    25794      -12     
===========================================
- Hits         20042    20031      -11     
+ Misses        5764     5763       -1     
Impacted Files Coverage Δ
src/vt/vrt/collection/balance/baselb/baselb.cc 81.76% <ø> (ø)
src/vt/vrt/collection/balance/baselb/baselb.h 100.00% <ø> (ø)
src/vt/vrt/collection/balance/greedylb/greedylb.cc 97.36% <ø> (-0.07%) ⬇️
src/vt/vrt/collection/balance/greedylb/greedylb.h 100.00% <ø> (ø)
.../vt/vrt/collection/balance/hierarchicallb/hierlb.h 100.00% <ø> (ø)
...c/vt/vrt/collection/balance/lb_invoke/lb_manager.h 100.00% <ø> (ø)
src/vt/vrt/collection/balance/rotatelb/rotatelb.h 100.00% <ø> (ø)
src/vt/vrt/collection/balance/stats_msg.h 97.01% <ø> (ø)
...vt/vrt/collection/balance/hierarchicallb/hierlb.cc 88.23% <80.00%> (-0.02%) ⬇️
.../vt/vrt/collection/balance/lb_invoke/lb_manager.cc 82.31% <95.83%> (+1.78%) ⬆️
... and 3 more

@lifflander
Copy link
Collaborator

Does this preempt #1059 ?

Copy link
Contributor

@JacobDomagala JacobDomagala left a comment

Choose a reason for hiding this comment

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

I was curious about the usage of lb_instances_ here but i can see that in PR #1059 it makes more sense when you allow LBType::NoLB to be a valid value

@PhilMiller
Copy link
Member Author

This likely leads to #1059 being closed, though I'm not ready to jump on that yet. I just wanted to get these easy and uncontroversial bits merged now, so that they can be built upon for #875, #965, and #1055

@PhilMiller PhilMiller merged commit cdb46a0 into develop Sep 25, 2020
@PhilMiller PhilMiller deleted the 1086-lb-refactor branch September 25, 2020 20:02
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.

LB refectoring on the path to persistent LB instances
3 participants