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

Barrier cleanup #1092

Merged
merged 5 commits into from
Jun 15, 2017
Merged

Barrier cleanup #1092

merged 5 commits into from
Jun 15, 2017

Conversation

garlick
Copy link
Member

@garlick garlick commented Jun 14, 2017

This PR modifies flux_barrier() to return a flux_future_t.

It also eliminates the libflux-barrier.so shared library and folds the one function in it into libflux. If the barrier module is not loaded, this function will return NULL with errno == ENOSYS.

Finally, it cleans up the function to eliminate exit-on-enomem behavior.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 666f56e on garlick:barrier_cleanup into ** on flux-framework:master**.

@codecov-io
Copy link

codecov-io commented Jun 14, 2017

Codecov Report

Merging #1092 into master will decrease coverage by 0.09%.
The diff coverage is 82.75%.

@@            Coverage Diff            @@
##           master    #1092     +/-   ##
=========================================
- Coverage   78.03%   77.94%   -0.1%     
=========================================
  Files         151      151             
  Lines       26105    26115     +10     
=========================================
- Hits        20371    20355     -16     
- Misses       5734     5760     +26
Impacted Files Coverage Δ
src/bindings/lua/flux-lua.c 81.67% <100%> (+0.05%) ⬆️
src/common/libflux/barrier.c 84.84% <80%> (ø)
src/common/libflux/keepalive.c 86.66% <0%> (-6.67%) ⬇️
src/common/libflux/ev_flux.c 97.56% <0%> (-2.44%) ⬇️
src/cmd/builtin/proxy.c 72.87% <0%> (-1.59%) ⬇️
src/broker/module.c 82.25% <0%> (-1.41%) ⬇️
src/broker/content-cache.c 72.98% <0%> (-1.31%) ⬇️
src/broker/modservice.c 79.2% <0%> (-1%) ⬇️
src/common/libflux/dispatch.c 85.54% <0%> (-0.79%) ⬇️
src/connectors/local/local.c 86.82% <0%> (-0.78%) ⬇️
... and 7 more

garlick added 5 commits June 14, 2017 22:44
Instead of building libbarrier.so as a standalone library,
fold it into libflux.so.
Problem: flux_barrier(), when called with a NULL name,
creates a unique barrier name based on a counter which
it stores in the flux_t handle under a key.  However,
the keys used to store and retrieve this state are
inconsistent so it starts from scratch each time, resulting
in non-unique barrier names.

Make the aux key names consistent.
Problem: flux_barrier() implementation calls xzmalloc() and
xasprintf() internally.  These function call exit(1) on
malloc failure and thus are inappropriate in a libflux function.

Use malloc() and related functions directly and return ENOMEM
to the caller if an out of memory error occurs.
Problem: flux_barrier () hardwires synchronous operation.

Have flux_barrier() return a future instead of integer success
code.  It can be completed with flux_future_get(f, NULL).

Convert users.
Relocate "tbarrier" from src/test to t/barrier, like all
the other sharness-drive C test programs.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3c384e6 on garlick:barrier_cleanup into ** on flux-framework:master**.

@grondo
Copy link
Contributor

grondo commented Jun 15, 2017

This looks like pretty good cleanup. I was going to say this misses the doc changes, but there's no doc for flux_barrier yet. So merge?

@garlick
Copy link
Member Author

garlick commented Jun 15, 2017

Sure, thanks.

@grondo grondo merged commit c5f839d into flux-framework:master Jun 15, 2017
@grondo grondo mentioned this pull request Aug 23, 2017
@garlick garlick deleted the barrier_cleanup branch September 6, 2017 15:53
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.

4 participants