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

fix kvs vs kvs-watch inconsistencies #1859

Merged
merged 8 commits into from Dec 5, 2018

Conversation

Projects
None yet
3 participants
@chu11
Copy link
Contributor

chu11 commented Dec 1, 2018

per discussion in #1855, there were inconsistencies in how the kvs & kvs-watch modules interacted regarding normalized keys and namespace prefixes. This resolves them by ensuring that after a key has been passed into kvs or kvs-watch, it is always processed / advertised / used as a normalized key and with the "ns:" prefix removed.

chu11 added some commits Nov 29, 2018

modules/kvs: Move kvs_util convenience library
Move kvs_util convenience library from modules/kvs to common/libkvs.
Rename kvs_util.h to kvs_util_private.h.  Also move kvs_util unit
tests.
modules/kvs: Refactor setroot keys generation
Generate keys for setroot after a kvstxn has completed processing.

Update unit tests appropriately.
modules/kvs: normalize keys on setroot events
When advertising keys in setroot events, normalize them
and strip ns: prefixes from the keys first.

Fixes #1855

@chu11 chu11 force-pushed the chu11:issue1855 branch from 039e5a6 to d5dfbca Dec 1, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Dec 1, 2018

Codecov Report

Merging #1859 into master will decrease coverage by 0.01%.
The diff coverage is 81.57%.

@@            Coverage Diff             @@
##           master    #1859      +/-   ##
==========================================
- Coverage   79.91%   79.89%   -0.02%     
==========================================
  Files         196      196              
  Lines       35198    35222      +24     
==========================================
+ Hits        28128    28142      +14     
- Misses       7070     7080      +10
Impacted Files Coverage Δ
src/modules/kvs/kvs.c 65.8% <ø> (+0.15%) ⬆️
src/modules/kvs/lookup.c 81.95% <ø> (ø) ⬆️
src/common/libkvs/kvs_util.c 90% <ø> (ø)
src/modules/kvs/cache.c 90.61% <ø> (ø) ⬆️
src/modules/kvs-watch/kvs-watch.c 76.15% <77.77%> (-0.14%) ⬇️
src/modules/kvs/kvstxn.c 77.33% <82.75%> (-0.17%) ⬇️
src/modules/barrier/barrier.c 76.55% <0%> (-2.07%) ⬇️
src/common/libflux/mrpc.c 86.71% <0%> (-1.18%) ⬇️
src/broker/modservice.c 79.8% <0%> (-0.97%) ⬇️
... and 3 more
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Dec 5, 2018

Did you want this to go in @chu11? LGTM.

@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Dec 5, 2018

yup, i think it's ready

@garlick garlick merged commit 81b5fe0 into flux-framework:master Dec 5, 2018

3 checks passed

codecov/patch 81.57% of diff hit (target 79.91%)
Details
codecov/project Absolute coverage decreased by -0.01% but relative coverage increased by +1.66% compared to 3466daa
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

chu11 added a commit to chu11/flux-core that referenced this pull request Jan 22, 2019

modules/kvs/kvstxn: Fix mem-leak in kvs merge
In kvstxn_create_empty(), the 'keys' array was created that
could be overwritten without being destroyed.  This was a corner
case in PR flux-framework#1859, when key generation was altered.

Fixes flux-framework#1944

chu11 added a commit to chu11/flux-core that referenced this pull request Jan 22, 2019

modules/kvs/kvstxn: Fix mem-leak in kvs merge
In kvstxn_create_empty(), the 'keys' array was created that
could be overwritten without being destroyed.  This was a corner
case in PR flux-framework#1859, when key generation was altered.

Fixes flux-framework#1944

chu11 added a commit to chu11/flux-core that referenced this pull request Jan 23, 2019

modules/kvs/kvstxn: Fix mem-leak in kvs merge
In kvstxn_create_empty(), the 'keys' array was created that
could be overwritten without being destroyed.  This was a corner
case in PR flux-framework#1859, when key generation was altered.

Fixes flux-framework#1944

chu11 added a commit to chu11/flux-core that referenced this pull request Jan 23, 2019

modules/kvs/kvstxn: Fix mem-leak in kvs merge
In kvstxn_create_empty(), the 'keys' array was created that
could be overwritten without being destroyed.  This was a corner
case in PR flux-framework#1859, when key generation was altered.

Fixes flux-framework#1944
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.