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

Broker cleanup 3 #1241

Merged
merged 11 commits into from
Oct 19, 2017
Merged

Broker cleanup 3 #1241

merged 11 commits into from
Oct 19, 2017

Conversation

morrone
Copy link
Contributor

@morrone morrone commented Oct 17, 2017

More cleanup and refactoring of the broker in preparation for issue #1236. The next step will likely be to break up boot_pmi() to split out the things that are really specific to PMI, and those that are not.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 78.779% when pulling d641072 on morrone:broker_cleanup_3 into 9f03dda on flux-framework:master.

@grondo
Copy link
Contributor

grondo commented Oct 17, 2017

restarted one travis job that hit "write error"

@codecov-io
Copy link

codecov-io commented Oct 17, 2017

Codecov Report

Merging #1241 into master will decrease coverage by 0.02%.
The diff coverage is 90.19%.

@@            Coverage Diff             @@
##           master    #1241      +/-   ##
==========================================
- Coverage   78.03%   78.01%   -0.03%     
==========================================
  Files         154      154              
  Lines       28936    28933       -3     
==========================================
- Hits        22581    22571      -10     
- Misses       6355     6362       +7
Impacted Files Coverage Δ
src/broker/broker.c 72.35% <85.71%> (-0.59%) ⬇️
src/broker/overlay.c 73.88% <93.33%> (+1.37%) ⬆️
src/common/libflux/keepalive.c 86.66% <0%> (-6.67%) ⬇️
src/common/libflux/mrpc.c 85.49% <0%> (-1.18%) ⬇️
src/cmd/flux-event.c 67.74% <0%> (-1.08%) ⬇️
src/broker/modservice.c 80.58% <0%> (-0.98%) ⬇️
src/common/libflux/response.c 83.73% <0%> (-0.82%) ⬇️
src/common/libkvs/kvs_txn.c 77.07% <0%> (-0.64%) ⬇️
src/common/libflux/handle.c 83.66% <0%> (-0.5%) ⬇️
src/common/libflux/future.c 88.31% <0%> (-0.47%) ⬇️
... and 7 more

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 78.76% when pulling 26e5d6f on morrone:broker_cleanup_3 into eebb5e7 on flux-framework:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 78.676% when pulling 26e5d6f on morrone:broker_cleanup_3 into eebb5e7 on flux-framework:master.

@garlick
Copy link
Member

garlick commented Oct 18, 2017

Good cleanup. Rebase?

Remove stray overlay_set_rank() that should have been removed in
PR #1234.
Rename calc_endpoint() to format_endpoint() to make its purpose
more clear.  It formats a string performing subsitutions much
like the sprintf().  There are no calculations performed.
Rename attr_get_overlay() to overlay_attr_get_cb() and move it
from broker.c into overlay.c.

"attr_get_overlay" sounds like an accessor that retrives an
overlay_t from an attr_t, but in reality it is a attr_get_f
callback function.

Move the new function to overlay.c from broker.c to further
isolate broker.c from the operations of the overlay
network.

Also an overlay_register_attrs() is added to overlay.c to register
all (both) of the overlay-specific attributes.  Again, this abstracts
more details away from broker.c.
Add FLUX_ATTRFLAG_READONLY to the "tbon.parent-endpoint" attribute,
which directly accesses the overlay_t's internal value.  We don't
provide a "set" callback anyway.
Relocate the tbon structure and its base calculations out of the
broker_ctx_t in broker.c and into the overlay_t in overlay.c.
This moves closer to having the overlay.c file encapsulate
the details the overlay network.
Remove the unused sigwatchers variable in broker_ctx_t.
Pid in broker_ctx_t is only used as a temporary variable, and therefore
does not need to exist.
Remove the unused "quiet" variable from broker_ctx_t.  We leave the
non-functional "-q" command line option in place to avoid changing the
command line interface.  But now it is completely clear that it is
ignored.
Move the timing of the boot_pmi() to the caller
of the function rather than internal to the function.
We will be breaking up boot_pmi() next, so getting that
out of the way will make things slightly easier.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 78.6% when pulling 1adc7bb on morrone:broker_cleanup_3 into 778e42b on flux-framework:master.

@garlick garlick merged commit 2095397 into flux-framework:master Oct 19, 2017
@morrone morrone deleted the broker_cleanup_3 branch October 19, 2017 23:12
@grondo grondo mentioned this pull request May 10, 2018
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.

5 participants