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

kvs: Misc cleanup & bug fixes #1244

Merged
merged 6 commits into from Oct 20, 2017

Conversation

Projects
None yet
4 participants
@chu11
Copy link
Contributor

chu11 commented Oct 20, 2017

Spliced out of a bigger PR for #1239. Nothing too exciting.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 20, 2017

LGTM

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 20, 2017

Codecov Report

Merging #1244 into master will increase coverage by 0.08%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master    #1244      +/-   ##
==========================================
+ Coverage   77.98%   78.06%   +0.08%     
==========================================
  Files         154      154              
  Lines       28961    28964       +3     
==========================================
+ Hits        22584    22610      +26     
+ Misses       6377     6354      -23
Impacted Files Coverage Δ
src/modules/kvs/kvs.c 63.72% <50%> (+0.2%) ⬆️
src/modules/kvs/commit.c 76.64% <75%> (+0.04%) ⬆️
src/cmd/flux-event.c 67.74% <0%> (-1.08%) ⬇️
src/common/libflux/message.c 81.25% <0%> (+0.11%) ⬆️
src/modules/connector-local/local.c 76.07% <0%> (+0.18%) ⬆️
src/common/libkvs/kvs.c 65.07% <0%> (+0.2%) ⬆️
src/common/libflux/future.c 89.25% <0%> (+0.46%) ⬆️
src/common/libflux/handle.c 84.15% <0%> (+0.49%) ⬆️
... and 4 more
@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 20, 2017

Coverage Status

Coverage increased (+0.03%) to 78.636% when pulling 943cf26 on chu11:misccleanup11 into 2095397 on flux-framework:master.

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Oct 20, 2017

bad coverage on diff, unsurprising given hard to reach error paths. hit two write errors on builds and a hang, unclear when/how builder hung. Restarting.

chu11 added some commits Oct 18, 2017

modules/kvs: Remove unnecessary comment
Remove no longer relevant comment about synchronous loads.  Synchronous
loads were removed in commit 19e1fb8.
modules/kvs: Fix error handling bug
On cache_entry_create_json() error, we should not return to the
caller.  Instead we should fallthrough and still call setroot() below.
modules/kvs/commit: Fix cleanup corner case
Handle both raw and json cache data when cleaning up in
commit_cleanup_dirty_cache_entry().

@chu11 chu11 force-pushed the chu11:misccleanup11 branch from 943cf26 to df5f572 Oct 20, 2017

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 20, 2017

Coverage Status

Changes Unknown when pulling df5f572 on chu11:misccleanup11 into ** on flux-framework:master**.

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Oct 20, 2017

rebased and re-pushed, couple of builds had write-errors, restarted them and seems everything has passed now

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Oct 20, 2017

Great - merging. Thanks!

@garlick garlick merged commit 117ea15 into flux-framework:master Oct 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 78.645%
Details

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

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.