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.

Sign up for GitHub

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

Jump to bottom

broker: misc cleanup / refactoring #2178

Merged
merged 7 commits into from Jun 6, 2019
Merged

broker: misc cleanup / refactoring #2178

merged 7 commits into from Jun 6, 2019

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Jun 6, 2019

While looking at #1036, fixed up a bunch of tiny issues I saw. Also refactored a few things for consistency. Note that some of these might eventually lead to more cleanup / refactoring (i.e. some type of ctx_init() type of the function), but I'll leave that for another day. I thought this was a good stopping point of cleanups.

chu11 added 7 commits June 5, 2019 20:54
@chu11
broker: Fix error message typo
ee70e8f
@chu11
broker: Fix code style per RFC7
547f07d
@chu11
broker: Do not exit on sigmask restore path
0680f74 
When trying to restore default sigmasks, do not log_err_exit()
on error, just fallthrough to continue cleanup path of broker.
@chu11
broker: Make create functions consistent
57f08ed 
For consistency, make all create functions (i.e. attr_create(),
heartbeat_create(), etc.) return ENOMEM on error.  Check for error
appropriately from callers.
@chu11
broker: Move ctx inits to same location
bbc926b 
Move several ctx initializations to same place in broker main.
@chu11
broker: rename shutdown_set_handle()
a7f346a 
Rename to shutdown_set_flux() for consistency to rest of broker
(i.e. content_cache_set_flux(), hello_set_flux(), etc.).
@chu11
broker: remove attr_unregister_handlers
d5b632e 
Move unregistration into attr_destroy().  Call attr_destroy() at
appropriate point in broker instead of attr_unregister_handlers().
@chu11
Copy link
Member Author

chu11 commented Jun 6, 2019

Hmmm, hit a valgrind issue in 1 run, but nothing obvious. Re-run

==31779== 
==31779== HEAP SUMMARY:
==31779==     in use at exit: 27,632 bytes in 70 blocks
==31779==   total heap usage: 25,667 allocs, 25,597 frees, 17,205,479 bytes allocated
==31779== 
==31779== LEAK SUMMARY:
==31779==    definitely lost: 0 bytes in 0 blocks
==31779==    indirectly lost: 0 bytes in 0 blocks
==31779==      possibly lost: 0 bytes in 0 blocks
==31779==    still reachable: 27,632 bytes in 70 blocks
==31779==         suppressed: 0 bytes in 0 blocks
==31779== Reachable blocks (those to which a pointer was found) are not shown.
==31779== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==31779== 
==31779== For counts of detected and suppressed errors, rerun with: -v
==31779== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
flux-start: 0 (pid 31778) Killed
not ok 1 - valgrind reports no new errors on 2 broker run

@garlick
Copy link
Member

garlick commented Jun 6, 2019

This all looks fine to me. Ready?

@chu11
Copy link
Member Author

chu11 commented Jun 6, 2019

yup

@garlick garlick merged commit c3f4723 into flux-framework:master Jun 6, 2019
@chu11 chu11 mentioned this pull request Jun 15, 2019
Merged
@garlick garlick mentioned this pull request Jun 15, 2019
Closed
@chu11 chu11 deleted the broker_cleanup1 branch June 5, 2021 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@chu11 @garlick