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: don't connect to enclosing instance #1798

Merged
merged 2 commits into from Nov 6, 2018

Conversation

Projects
None yet
3 participants
@garlick
Copy link
Member

garlick commented Nov 4, 2018

This PR removes some code in the broker that attempted to set up for the content-cache to use its enclosing instance as a tertiary level of content storage. That feature was not fully implemented, is not motivated by a use case, and creates a complication for test environments, as pointed out in #1784.

There is a corresponding PR to make this feature optional instead of mandatory in RFC 10 in flux-framework/rfc#141.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 4, 2018

Codecov Report

Merging #1798 into master will increase coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1798      +/-   ##
==========================================
+ Coverage   79.61%   79.65%   +0.04%     
==========================================
  Files         186      186              
  Lines       34562    34554       -8     
==========================================
+ Hits        27517    27525       +8     
+ Misses       7045     7029      -16
Impacted Files Coverage Δ
src/broker/broker.c 77% <ø> (-0.05%) ⬇️
src/broker/content-cache.c 74.24% <ø> (-0.12%) ⬇️
src/modules/kvs/kvs.c 65.71% <0%> (+0.15%) ⬆️
src/broker/module.c 83.83% <0%> (+0.27%) ⬆️
src/cmd/flux-module.c 85.58% <0%> (+0.3%) ⬆️
src/broker/modservice.c 79.8% <0%> (+0.96%) ⬆️
src/modules/connector-local/local.c 74.96% <0%> (+1.02%) ⬆️
src/common/libflux/mrpc.c 86.69% <0%> (+1.14%) ⬆️

@garlick garlick force-pushed the garlick:no_open_parent branch from 5b3b8d0 to 6a449a4 Nov 6, 2018

@garlick

This comment has been minimized.

Copy link
Member Author

garlick commented Nov 6, 2018

Rebased on current master.

garlick added some commits Nov 4, 2018

broker: don't open parent FLUX_URI
Problem: broker unnecessarily connects to enclosing instance,
which makes tests sensitive to environment.

Don't connect to parent, and don't pass open handle to
the content cache.

Fixes #1784
broker/content-cache: drop cache_set_enclosing_flux()
Drop unused code for setting flux_t handle of enclosing
instance.

@garlick garlick force-pushed the garlick:no_open_parent branch from 6a449a4 to db9d240 Nov 6, 2018

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Nov 6, 2018

Thanks for fixing this!

@grondo grondo merged commit 1956120 into flux-framework:master Nov 6, 2018

3 checks passed

codecov/patch Coverage not affected when comparing 5e4ff81...db9d240
Details
codecov/project 79.65% (+0.04%) compared to 5e4ff81
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.