From 67f05b12d13a76f2890edb4733a45ab5a2a178ac Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 1 Nov 2017 16:35:12 -0700 Subject: [PATCH] modules/kvs: Remove cache_lookup_and_get_treeobj The function was more widely used before, but is now only used in two locations. Remove function and replace with calls to other functions. Remove unit tests for cache_lookup_and_get_treeobj(). Adjust callers appropriately. --- src/modules/kvs/cache.c | 8 -------- src/modules/kvs/cache.h | 9 --------- src/modules/kvs/commit.c | 25 +++++++++++++++++++------ src/modules/kvs/test/cache.c | 8 -------- 4 files changed, 19 insertions(+), 31 deletions(-) diff --git a/src/modules/kvs/cache.c b/src/modules/kvs/cache.c index 350c6fd9445a..e2a43774bad7 100644 --- a/src/modules/kvs/cache.c +++ b/src/modules/kvs/cache.c @@ -294,14 +294,6 @@ struct cache_entry *cache_lookup (struct cache *cache, const char *ref, return hp; } -json_t *cache_lookup_and_get_treeobj (struct cache *cache, - const char *ref, - int current_epoch) -{ - struct cache_entry *hp = cache_lookup (cache, ref, current_epoch); - return cache_entry_get_valid (hp) ? cache_entry_get_treeobj (hp) : NULL; -} - void cache_insert (struct cache *cache, const char *ref, struct cache_entry *hp) { int rc = zhash_insert (cache->zh, ref, hp); diff --git a/src/modules/kvs/cache.h b/src/modules/kvs/cache.h index b5c605fa5635..e83c1a59d8ed 100644 --- a/src/modules/kvs/cache.h +++ b/src/modules/kvs/cache.h @@ -105,15 +105,6 @@ void cache_destroy (struct cache *cache); struct cache_entry *cache_lookup (struct cache *cache, const char *ref, int current_epoch); -/* Look up a cache entry and get treeobj of cache entry only if entry - * contains a valid treeobj. This is a convenience function that is - * effectively successful if calls to cache_lookup() and - * cache_entry_get_treeobj() are both successful. - */ -json_t *cache_lookup_and_get_treeobj (struct cache *cache, - const char *ref, - int current_epoch); - /* Insert an entry in the cache by blobref 'ref'. * Ownership of the cache entry is transferred to the cache. */ diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index ce4ad2ae38c3..db8ff1323b44 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -557,6 +557,7 @@ static int commit_link_dirent (commit_t *c, int current_epoch, } else if (treeobj_is_dir (dir_entry)) { subdir = dir_entry; } else if (treeobj_is_dirref (dir_entry)) { + struct cache_entry *hp; const char *ref; int refcount; @@ -577,13 +578,17 @@ static int commit_link_dirent (commit_t *c, int current_epoch, goto done; } - if (!(subdir = cache_lookup_and_get_treeobj (c->cm->cache, - ref, - current_epoch))) { + if (!(hp = cache_lookup (c->cm->cache, ref, current_epoch)) + || !cache_entry_get_valid (hp)) { *missing_ref = ref; goto success; /* stall */ } + if (!(subdir = cache_entry_get_treeobj (hp))) { + saved_errno = ENOTRECOVERABLE; + goto done; + } + /* do not corrupt store by modifying orig. */ if (!(subdir = treeobj_copy (subdir))) { saved_errno = errno; @@ -693,6 +698,7 @@ commit_process_t commit_process (commit_t *c, { /* Make a copy of the root directory. */ + struct cache_entry *hp; json_t *rootdir; /* Caller didn't call commit_iter_missing_refs() */ @@ -701,9 +707,11 @@ commit_process_t commit_process (commit_t *c, c->state = COMMIT_STATE_LOAD_ROOT; - if (!(rootdir = cache_lookup_and_get_treeobj (c->cm->cache, - rootdir_ref, - current_epoch))) { + if (!(hp = cache_lookup (c->cm->cache, + rootdir_ref, + current_epoch)) + || !cache_entry_get_valid (hp)) { + if (zlist_push (c->missing_refs_list, (void *)rootdir_ref) < 0) { c->errnum = ENOMEM; @@ -712,6 +720,11 @@ commit_process_t commit_process (commit_t *c, goto stall_load; } + if (!(rootdir = cache_entry_get_treeobj (hp))) { + c->errnum = ENOTRECOVERABLE; + return COMMIT_PROCESS_ERROR; + } + if (!(c->rootcpy = treeobj_copy (rootdir))) { c->errnum = errno; return COMMIT_PROCESS_ERROR; diff --git a/src/modules/kvs/test/cache.c b/src/modules/kvs/test/cache.c index 4629492c6e35..d2b2c1bc26e3 100644 --- a/src/modules/kvs/test/cache.c +++ b/src/modules/kvs/test/cache.c @@ -600,12 +600,8 @@ void cache_expiration_tests (void) "cache contains 1 entry after insert"); ok (cache_lookup (cache, "yyy1", 0) == NULL, "cache_lookup of wrong hash fails"); - ok (cache_lookup_and_get_treeobj (cache, "yyy1", 0) == NULL, - "cache_lookup_and_get_treeobj of wrong hash fails"); ok ((e2 = cache_lookup (cache, "xxx1", 42)) != NULL, "cache_lookup of correct hash works (last use=42)"); - ok (cache_lookup_and_get_treeobj (cache, "xxx1", 0) == NULL, - "cache_lookup_and_get_treeobj of correct hash, but non valid entry fails"); ok (cache_entry_get_treeobj (e2) == NULL, "no treeobj object found"); ok (cache_count_entries (cache) == 1, @@ -642,10 +638,6 @@ void cache_expiration_tests (void) ok ((otmp = cache_entry_get_treeobj (e4)) != NULL, "cache_entry_get_treeobj found entry"); otest = treeobj_create_val ("foo", 3); - ok (json_equal (otmp, otest) == 1, - "expected treeobj object found"); - ok ((otmp = cache_lookup_and_get_treeobj (cache, "xxx2", 0)) != NULL, - "cache_lookup_and_get_treeobj of correct hash and valid entry works"); ok (json_equal (otmp, otest) == 1, "expected treeobj object found"); json_decref (otest);