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

kvs: store data pointed by valref raw and unencoded in content store #1214

Merged
merged 9 commits into from
Sep 28, 2017
Merged

kvs: store data pointed by valref raw and unencoded in content store #1214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dad5b37
modules/kvs/cache: Refactor internal structure
chu11 Sep 25, 2017
1ed68cc
modules/kvs/cache: Refactor cache_entry_create
chu11 Sep 22, 2017
4a0c7e7
modules/kvs/cache: Support cache data type
chu11 Sep 25, 2017
31db337
modules/kvs/cache: Add raw cache entry support
chu11 Sep 22, 2017
8218587
modules/kvs/lookup: Update for raw opaque data
chu11 Sep 19, 2017
df34e21
libkvs/treeobj: Remove treeobj_create_val_base64
chu11 Sep 27, 2017
4fdefb9
modules/kvs/commit: Update for raw opaque data
chu11 Sep 21, 2017
9428d33
modules/kvs: Update main KVS module for raw data
chu11 Sep 27, 2017
3e3c6ed
test/kvs: Add kvs raw unit tests
chu11 Sep 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 0 additions & 28 deletions src/common/libkvs/test/treeobj.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,33 +196,6 @@ void test_val (void)
json_decref (val2);
}

void test_val_base64 (void)
{
json_t *val;
char *base64 = "NDI="; /* 42 w/o ending NUL byte*/
char *outbuf;
int outlen;

ok ((val = treeobj_create_val_base64 (base64)) != NULL,
"treeobj_create_val_base64 works");
diag_json (val);
ok (treeobj_is_val (val),
"treeobj_is_value returns true");
ok (treeobj_decode_val (val, (void **)&outbuf, &outlen) == 0,
"treeobj_decode_val works");
ok (outlen == 2,
"and returned correct size");
ok (memcmp ("42", outbuf, 2) == 0,
"and returned correct data");
free (outbuf);

errno = 0;
ok (treeobj_create_val_base64 (NULL) == NULL && errno == EINVAL,
"treeobj_create_val_base64 NULL fails ");

json_decref (val);
}

void test_dirref (void)
{
json_t *dirref;
Expand Down Expand Up @@ -563,7 +536,6 @@ int main(int argc, char** argv)

test_valref ();
test_val ();
test_val_base64 ();
test_dirref ();
test_dir ();
test_copy_dir ();
Expand Down
20 changes: 2 additions & 18 deletions src/common/libkvs/treeobj.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,30 +380,14 @@ json_t *treeobj_create_val (const void *data, int len)
}
base64_encode_block (xdata, &xlen, data, len);

if (!(obj = treeobj_create_val_base64 (xdata)))
goto done;

done:
free (xdata);
return obj;
}

json_t *treeobj_create_val_base64 (const char *data)
{
json_t *obj = NULL;

if (!data) {
errno = EINVAL;
return NULL;
}

if (!(obj = json_pack ("{s:i s:s s:s}", "ver", treeobj_version,
"type", "val",
"data", data))) {
"data", xdata))) {
errno = ENOMEM;
goto done;
}
done:
free (xdata);
return obj;
}

Expand Down
4 changes: 1 addition & 3 deletions src/common/libkvs/treeobj.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,11 @@
/* Create a treeobj
* valref, dirref: if blobref is NULL, treeobj_append_blobref()
* must be called before object is valid.
* val & val_base64: copies argument (caller retains ownership)
* val_base64: user supplies base64 string
* val: copies argument (caller retains ownership)
* Return JSON object on success, NULL on failure with errno set.
*/
json_t *treeobj_create_symlink (const char *target);
json_t *treeobj_create_val (const void *data, int len);
json_t *treeobj_create_val_base64 (const char *data);
json_t *treeobj_create_valref (const char *blobref);
json_t *treeobj_create_dir (void);
json_t *treeobj_create_dirref (const char *blobref);
Expand Down
164 changes: 135 additions & 29 deletions src/modules/kvs/cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@
struct cache_entry {
waitqueue_t *waitlist_notdirty;
waitqueue_t *waitlist_valid;
json_t *o; /* value object */
void *data; /* value object/data */
int len;
cache_data_type_t type; /* what does data point to */
int lastuse_epoch; /* time of last use for cache expiry */
uint8_t dirty:1;
};
Expand All @@ -60,31 +62,80 @@ struct cache {
zhash_t *zh;
};

struct cache_entry *cache_entry_create (json_t *o)
struct cache_entry *cache_entry_create (void)
{
struct cache_entry *hp = calloc (1, sizeof (*hp));
if (!hp) {
errno = ENOMEM;
return NULL;
}
hp->type = CACHE_DATA_TYPE_NONE;
return hp;
}

struct cache_entry *cache_entry_create_json (json_t *o)
{
struct cache_entry *hp = cache_entry_create ();
if (!hp)
return NULL;
if (o)
hp->o = o;
hp->data = o;
hp->type = CACHE_DATA_TYPE_JSON;
return hp;
}

struct cache_entry *cache_entry_create_raw (void *data, int len)
{
struct cache_entry *hp;

if ((data && len <= 0) || (!data && len)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content cache allows a size = 0 blob to be stored. Probably we should allow it in the KVS and add test coverage. Not sure if it worked before, but this would prevent it I guess.

I'll review more closely tomorrow - just noticed that on a quick skim.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the intent was if len == 0, data should == NULL. Would this be ok?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh duh, sorry, I guess I didn't look closely enough there!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open a separate bug on ability to store zero length data under a key as there do seem to be some issues there, unrelated to this PR.

errno = EINVAL;
return NULL;
}

if (!(hp = cache_entry_create ()))
return NULL;
if (data) {
hp->data = data;
hp->len = len;
}
hp->type = CACHE_DATA_TYPE_RAW;
return hp;
}

int cache_entry_type (struct cache_entry *hp, cache_data_type_t *t)
{
if (hp) {
if (t)
(*t) = hp->type;
return 0;
}
return -1;
}

bool cache_entry_is_type_json (struct cache_entry *hp)
{
return (hp && hp->type == CACHE_DATA_TYPE_JSON);
}

bool cache_entry_is_type_raw (struct cache_entry *hp)
{
return (hp && hp->type == CACHE_DATA_TYPE_RAW);
}

bool cache_entry_get_valid (struct cache_entry *hp)
{
return (hp && hp->o != NULL);
return (hp && hp->data != NULL);
}

bool cache_entry_get_dirty (struct cache_entry *hp)
{
return (hp && hp->o && hp->dirty);
return (hp && hp->data && hp->dirty);
}

int cache_entry_set_dirty (struct cache_entry *hp, bool val)
{
if (hp && hp->o) {
if (hp && hp->data) {
if ((val && hp->dirty) || (!val && !hp->dirty))
; /* no-op */
else if (val && !hp->dirty)
Expand All @@ -106,7 +157,7 @@ int cache_entry_set_dirty (struct cache_entry *hp, bool val)

int cache_entry_clear_dirty (struct cache_entry *hp)
{
if (hp && hp->o) {
if (hp && hp->data) {
if (hp->dirty
&& (!hp->waitlist_notdirty
|| !wait_queue_length (hp->waitlist_notdirty)))
Expand All @@ -118,7 +169,7 @@ int cache_entry_clear_dirty (struct cache_entry *hp)

int cache_entry_force_clear_dirty (struct cache_entry *hp)
{
if (hp && hp->o) {
if (hp && hp->data) {
if (hp->dirty) {
if (hp->waitlist_notdirty) {
wait_queue_destroy (hp->waitlist_notdirty);
Expand All @@ -133,29 +184,75 @@ int cache_entry_force_clear_dirty (struct cache_entry *hp)

json_t *cache_entry_get_json (struct cache_entry *hp)
{
if (!hp || !hp->o)
if (!hp || !hp->data || hp->type != CACHE_DATA_TYPE_JSON)
return NULL;
return hp->o;
return hp->data;
}

int cache_entry_set_json (struct cache_entry *hp, json_t *o)
{
if (hp) {
if ((o && hp->o) || (!o && !hp->o)) {
json_decref (o); /* no-op, 'o' is assumed identical to hp->o */
} else if (o && !hp->o) {
hp->o = o;
if (hp
&& (hp->type == CACHE_DATA_TYPE_NONE
|| hp->type == CACHE_DATA_TYPE_JSON)) {
if ((o && hp->data) || (!o && !hp->data)) {
json_decref (o); /* no-op, 'o' is assumed identical to hp->data */
} else if (o && !hp->data) {
hp->data = o;
if (hp->waitlist_valid) {
if (wait_runqueue (hp->waitlist_valid) < 0) {
/* set back to orig */
hp->o = NULL;
hp->data = NULL;
return -1;
}
}
} else if (!o && hp->o) {
json_decref (hp->o);
hp->o = NULL;
} else if (!o && hp->data) {
json_decref (hp->data);
hp->data = NULL;
}
hp->type = CACHE_DATA_TYPE_JSON;
return 0;
}
return -1;
}

void *cache_entry_get_raw (struct cache_entry *hp, int *len)
{
if (!hp || !hp->data || hp->type != CACHE_DATA_TYPE_RAW)
return NULL;
if (len)
(*len) = hp->len;
return hp->data;
}

int cache_entry_set_raw (struct cache_entry *hp, void *data, int len)
{
if ((data && len <= 0) || (!data && len)) {
errno = EINVAL;
return -1;
}

if (hp
&& (hp->type == CACHE_DATA_TYPE_NONE
|| hp->type == CACHE_DATA_TYPE_RAW)) {
if ((data && hp->data) || (!data && !hp->data)) {
free (data); /* no-op, 'data' is assumed identical to hp->data */
} else if (data && !hp->data) {
hp->data = data;
hp->len = len;
if (hp->waitlist_valid) {
if (wait_runqueue (hp->waitlist_valid) < 0) {
/* set back to orig */
hp->data = NULL;
hp->len = 0;
return -1;
}
}
} else if (!data && hp->data) {
free (hp->data);
hp->data = NULL;
hp->len = 0;
}
hp->type = CACHE_DATA_TYPE_RAW;
return 0;
}
return -1;
Expand All @@ -165,8 +262,12 @@ void cache_entry_destroy (void *arg)
{
struct cache_entry *hp = arg;
if (hp) {
if (hp->o)
json_decref (hp->o);
if (hp->data) {
if (hp->type == CACHE_DATA_TYPE_JSON)
json_decref (hp->data);
else if (hp->type == CACHE_DATA_TYPE_RAW)
free (hp->data);
}
if (hp->waitlist_notdirty)
wait_queue_destroy (hp->waitlist_notdirty);
if (hp->waitlist_valid)
Expand Down Expand Up @@ -299,15 +400,20 @@ int cache_get_stats (struct cache *cache, tstat_t *ts, int *sizep,
while ((ref = zlist_pop (keys))) {
hp = zhash_lookup (cache->zh, ref);
if (cache_entry_get_valid (hp)) {
/* must pass JSON_ENCODE_ANY, object could be anything */
char *s = json_dumps (hp->o, JSON_ENCODE_ANY);
int obj_size;
if (!s) {
saved_errno = ENOMEM;
goto cleanup;
int obj_size = 0;

if (hp->type == CACHE_DATA_TYPE_JSON) {
/* must pass JSON_ENCODE_ANY, object could be anything */
char *s = json_dumps (hp->data, JSON_ENCODE_ANY);
if (!s) {
saved_errno = ENOMEM;
goto cleanup;
}
obj_size = strlen (s);
free (s);
}
obj_size = strlen (s);
free (s);
else if (hp->type == CACHE_DATA_TYPE_RAW)
obj_size = hp->len;
size += obj_size;
tstat_push (ts, obj_size);
} else
Expand Down
Loading