From 2aba6b7b90b0ac137460c003c1d87ed34b018d84 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 27 Jul 2017 11:03:05 -0700 Subject: [PATCH 1/8] libkvs/txn: add missing copyright header --- src/common/libkvs/kvs_txn.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c index 49e96edeb727..d238e8ae0b61 100644 --- a/src/common/libkvs/kvs_txn.c +++ b/src/common/libkvs/kvs_txn.c @@ -1,3 +1,27 @@ +/*****************************************************************************\ + * Copyright (c) 2017 Lawrence Livermore National Security, LLC. Produced at + * the Lawrence Livermore National Laboratory (cf, AUTHORS, DISCLAIMER.LLNS). + * LLNL-CODE-658032 All rights reserved. + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the license, or (at your option) + * any later version. + * + * Flux is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the IMPLIED WARRANTY OF MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the terms and conditions of the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. + * See also: http://www.gnu.org/licenses/ +\*****************************************************************************/ + #if HAVE_CONFIG_H #include "config.h" #endif From a306f94cf663f0f5f76d1434b7faaf13a987205b Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 2 Aug 2017 16:30:35 -0700 Subject: [PATCH 2/8] modules/kvs/test: Fix whitespace/tabbing --- src/modules/kvs/test/commit.c | 2 +- src/modules/kvs/test/fence.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index 3126fe5bf4c6..d2382c3ed885 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -106,7 +106,7 @@ void commit_mgr_basic_tests (void) ops = json_array (); ops_append (ops, "key1", "1"); - + ok (fence_add_request_data (f, ops) == 0, "fence_add_request_data add works"); diff --git a/src/modules/kvs/test/fence.c b/src/modules/kvs/test/fence.c index 7c230e444ef8..91b3a2676973 100644 --- a/src/modules/kvs/test/fence.c +++ b/src/modules/kvs/test/fence.c @@ -147,8 +147,8 @@ void ops_tests (void) "initial fence_get_json_ops call works"); ops = json_array (); - json_array_append_new (ops, json_string ("A")); - json_array_append_new (ops, json_string ("B")); + json_array_append_new (ops, json_string ("A")); + json_array_append_new (ops, json_string ("B")); ok (json_equal (ops, o) == true, "fence_get_json_ops match"); From 2deb30cbccc38e6e8625b476451ff443b010202b Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 8 Aug 2017 10:56:38 -0700 Subject: [PATCH 3/8] modules/kvs/test: Add new commit test In commit_basic_commit_process_test_multiple_fences(), make one of the commits in a directory vs the root dir, to mix up the workload. --- src/modules/kvs/test/commit.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index d2382c3ed885..efe58dd935e8 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -472,7 +472,7 @@ void commit_basic_commit_process_test_multiple_fences (void) "commit_mgr_create works"); create_ready_commit (cm, "fence1", "key1", "1", 0); - create_ready_commit (cm, "fence2", "key2", "2", 0); + create_ready_commit (cm, "fence2", "dir.key2", "2", 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -508,7 +508,8 @@ void commit_basic_commit_process_test_multiple_fences (void) ok (commit_iter_dirty_cache_entries (c, cache_count_cb, &count) == 0, "commit_iter_dirty_cache_entries works for dirty cache entries"); - ok (count == 1, + /* why two? 1 for root (new dir added), 1 for dir.key2 (a new dir) */ + ok (count == 2, "correct number of cache entries were dirty"); ok (commit_process (c, 1, rootref) == COMMIT_PROCESS_FINISHED, @@ -518,7 +519,7 @@ void commit_basic_commit_process_test_multiple_fences (void) "commit_get_newroot_ref returns != NULL when processing complete"); verify_value (cache, newroot, "key1", "1"); - verify_value (cache, newroot, "key2", "2"); + verify_value (cache, newroot, "dir.key2", "2"); commit_mgr_remove_commit (cm, c); From f1117e1be2fb39f5934583a14ca47f8284b6a21d Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 8 Aug 2017 11:53:55 -0700 Subject: [PATCH 4/8] modules/kvs/test: Add missing tests Add missing function call to run some commit unit tests. --- src/modules/kvs/test/commit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index efe58dd935e8..05ba0d334c98 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -1608,6 +1608,7 @@ int main (int argc, char *argv[]) * "normal" situation and is tested throughout */ commit_process_error_callbacks (); + commit_process_error_callbacks_partway (); commit_process_invalid_operation (); commit_process_invalid_hash (); commit_process_follow_link (); From a362f0001f151652466d12950f9dfa67b5cbbcd3 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 7 Aug 2017 14:28:13 -0700 Subject: [PATCH 5/8] modules/kvs: Fix invalid cleanup on error json_array_get() returns a borrowed reference, not a copy, so on error path the json object should not have its reference count decreased. --- src/modules/kvs/fence.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/kvs/fence.c b/src/modules/kvs/fence.c index 33e0a139d651..d684b07fe24f 100644 --- a/src/modules/kvs/fence.c +++ b/src/modules/kvs/fence.c @@ -125,7 +125,6 @@ int fence_add_request_data (fence_t *f, json_t *ops) for (i = 0; i < json_array_size (ops); i++) { if ((op = json_array_get (ops, i))) if (json_array_append (f->ops, op) < 0) { - json_decref (op); errno = ENOMEM; return -1; } From f2ac952ec19133deea4dc5c990b38d80e0b3caea Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 7 Aug 2017 16:15:29 -0700 Subject: [PATCH 6/8] modules/kvs: Remove unnecessary cleanup Remove cleanup on error path that is unnecessary. The item_callback_list in this particular area of code will always be empty if it has reached this particular error path. --- src/modules/kvs/commit.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index 9c9fd059b9dd..f1e3ba97e19f 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -481,9 +481,6 @@ commit_process_t commit_process (commit_t *c, } if (!(c->rootcpy = json_copy (rootdir))) { - const char *tmpref; - /* empty item_callback_list to prevent mistakes later */ - while ((tmpref = zlist_pop (c->item_callback_list))); c->errnum = ENOMEM; return COMMIT_PROCESS_ERROR; } From 190eee33ad010cb5a0f410a20f47eecd46aab858 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 16 Aug 2017 15:12:16 -0700 Subject: [PATCH 7/8] bindings/lua: Fix error message typos --- src/bindings/lua/flux-lua.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bindings/lua/flux-lua.c b/src/bindings/lua/flux-lua.c index 084269569b96..d5d4c6cd219d 100644 --- a/src/bindings/lua/flux-lua.c +++ b/src/bindings/lua/flux-lua.c @@ -368,7 +368,7 @@ int l_flux_kvs_put (lua_State *L) json_object_put (o); } if (rc < 0) - return lua_pusherror (L, "kvsdir_put (%s): %s", + return lua_pusherror (L, "kvs_put (%s): %s", key, (char *)flux_strerror (errno)); lua_pushboolean (L, true); From 331763d4924f9934be2b84a15f1a1dcc4ae99c8b Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 16 Aug 2017 17:13:53 -0700 Subject: [PATCH 8/8] t/kvs: Fix corner case in watch tests Fix bug which error in watch callbacks was not detected and tests could still pass. --- t/kvs/watch.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/t/kvs/watch.c b/t/kvs/watch.c index 5799537f6fac..fc2a5a1e1b38 100644 --- a/t/kvs/watch.c +++ b/t/kvs/watch.c @@ -61,6 +61,7 @@ typedef struct { int nil_count; int stable_count; int last_val; + int errcount; } thd_t; static void signal_ready (void) @@ -101,16 +102,19 @@ static int mt_watch_cb (const char *k, const char *json_str, void *arg, int errn if (errnum != 0) { log_errn (errnum, "%d: %s", t->n, __FUNCTION__); + t->errcount++; return -1; } if (!(obj = json_loads (json_str, JSON_DECODE_ANY, NULL))) { log_msg ("%d: %s failed to decode value", t->n, __FUNCTION__); + t->errcount++; return -1; } val = json_integer_value (obj); if (val == t->last_val) { log_msg ("%d: %s: called with same value as last time: %d", t->n, __FUNCTION__, val); + t->errcount++; return -1; } t->last_val = val; @@ -130,6 +134,7 @@ static int mt_watchnil_cb (const char *k, const char *json_str, void *arg, int e thd_t *t = arg; if (errnum != ENOENT) { log_errn (errnum, "%d: %s", t->n, __FUNCTION__); + t->errcount++; return -1; } t->nil_count++; @@ -144,6 +149,7 @@ static int mt_watchstable_cb (const char *k, const char *json_ttr, void *arg, in if (errnum != 0) { log_errn (errnum, "%d: %s", t->n, __FUNCTION__); + t->errcount++; return -1; } t->stable_count++; @@ -254,6 +260,10 @@ void test_mt (int argc, char **argv) for (i = 0; i < nthreads; i++) { if ((rc = pthread_join (thd[i].tid, NULL))) log_errn (rc, "pthread_join"); + if (thd[i].errcount != 0) { + log_msg ("%d: error occurred inside callback function", i); + errors++; + } if (thd[i].nil_count != 1) { log_msg ("%d: nil callback called %d times (expected one)", i, thd[i].nil_count);