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

libkvs: Require txn argument in flux_kvs_commit() and flux_kvs_fence() #1351

Merged
merged 2 commits into from
Mar 7, 2018

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Mar 6, 2018

No description provided.

@chu11
Copy link
Member Author

chu11 commented Mar 6, 2018

Actually, just noticed a corner case. Wait till i re-push.

@chu11 chu11 force-pushed the issue1350 branch 2 times, most recently from 6a65257 to b75a42a Compare March 6, 2018 23:24
In wreck_pmi_barrier_enter(), remember to set f == NULL in the
error path so the correct return code is returned.
In flux_kvs_commit() and flux_kvs_fence(), require txn to always
be passed in.  Return EINVAL if a NULL is passed in.

Add unit tests to test for corner case.

Adjust callers appropriately.

Fixes flux-framework#1350
@chu11
Copy link
Member Author

chu11 commented Mar 6, 2018

Just re-pushed with a corner case fix patch.

As a note, I preserved behavior of wreck_pmi_barrier_enter(), which is that if the barrier_txn was not-NULL and flux_kvs_fence() fails, the barrier_txn is not destroyed.

@garlick
Copy link
Member

garlick commented Mar 6, 2018

That looks much nicer. Will merge when travis is done.

@codecov-io
Copy link

codecov-io commented Mar 7, 2018

Codecov Report

Merging #1351 into master will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1351      +/-   ##
==========================================
+ Coverage    78.5%   78.51%   +0.01%     
==========================================
  Files         162      162              
  Lines       29686    29690       +4     
==========================================
+ Hits        23304    23312       +8     
+ Misses       6382     6378       -4
Impacted Files Coverage Δ
src/common/libkvs/kvs_commit.c 77.41% <66.66%> (+3.88%) ⬆️
src/modules/wreck/wrexecd.c 76.08% <66.66%> (-0.11%) ⬇️
src/modules/connector-local/local.c 72.74% <0%> (-1.85%) ⬇️
src/broker/content-cache.c 73.43% <0%> (-0.65%) ⬇️
src/bindings/lua/flux-lua.c 81.08% <0%> (-0.09%) ⬇️
src/common/libflux/message.c 81.25% <0%> (+0.11%) ⬆️
src/common/libflux/reactor.c 93.73% <0%> (+0.28%) ⬆️
src/common/libflux/future.c 88.31% <0%> (+0.46%) ⬆️
src/common/libflux/handle.c 84.15% <0%> (+0.49%) ⬆️
src/common/libflux/msg_handler.c 87.36% <0%> (+0.72%) ⬆️
... and 5 more

@garlick garlick merged commit c6c48fd into flux-framework:master Mar 7, 2018
@grondo grondo mentioned this pull request May 10, 2018
@chu11 chu11 deleted the issue1350 branch June 5, 2021 17:04
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.

3 participants