Skip to content

Commit c205af7

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #6615 from chu11/kvs_cleanup3
kvs: misc cleanups
2 parents ef29673 + a1b2c06 commit c205af7

File tree

11 files changed

+122
-268
lines changed

11 files changed

+122
-268
lines changed

‎src/modules/kvs/kvs.c‎

Lines changed: 53 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "src/common/libutil/tstat.h"
3131
#include "src/common/libutil/timestamp.h"
3232
#include "src/common/libutil/errprintf.h"
33+
#include "src/common/libutil/errno_safe.h"
3334
#include "src/common/libkvs/treeobj.h"
3435
#include "src/common/libkvs/kvs_checkpoint.h"
3536
#include "src/common/libkvs/kvs_txn_private.h"
@@ -328,7 +329,6 @@ static void getroot_completion (flux_future_t *f, void *arg)
328329
uint32_t owner;
329330
const char *ref;
330331
struct kvsroot *root;
331-
int save_errno;
332332

333333
msg = flux_future_aux_get (f, "msg");
334334
assert (msg);
@@ -361,7 +361,7 @@ static void getroot_completion (flux_future_t *f, void *arg)
361361
}
362362

363363
if (event_subscribe (ctx, ns) < 0) {
364-
save_errno = errno;
364+
int save_errno = errno;
365365
kvsroot_mgr_remove_root (ctx->krm, ns);
366366
errno = save_errno;
367367
flux_log_error (ctx->h, "%s: event_subscribe", __FUNCTION__);
@@ -380,7 +380,6 @@ static void getroot_completion (flux_future_t *f, void *arg)
380380
goto error;
381381
}
382382

383-
flux_msg_destroy (msg);
384383
flux_future_destroy (f);
385384
return;
386385

@@ -394,18 +393,15 @@ static void getroot_completion (flux_future_t *f, void *arg)
394393
* will deal with the success case.
395394
*/
396395
request_tracking_remove (ctx, msg);
397-
flux_msg_destroy (msg);
398396
flux_future_destroy (f);
399397
}
400398

401399
static int getroot_request_send (struct kvs_ctx *ctx,
402400
const char *ns,
403401
flux_msg_handler_t *mh,
404-
const flux_msg_t *msg,
405-
lookup_t *lh)
402+
const flux_msg_t *msg)
406403
{
407404
flux_future_t *f = NULL;
408-
flux_msg_t *msgcpy = NULL;
409405
int saved_errno;
410406

411407
if (!(f = flux_rpc_pack (ctx->h,
@@ -416,20 +412,12 @@ static int getroot_request_send (struct kvs_ctx *ctx,
416412
"namespace", ns)))
417413
goto error;
418414

419-
if (!(msgcpy = flux_msg_copy (msg, true))) {
420-
flux_log_error (ctx->h, "%s: flux_msg_copy", __FUNCTION__);
421-
goto error;
422-
}
423-
424-
if (lh
425-
&& flux_msg_aux_set (msgcpy, "lookup_handle", lh, NULL) < 0) {
426-
flux_log_error (ctx->h, "%s: flux_msg_aux_set", __FUNCTION__);
427-
goto error;
428-
}
429-
430-
/* we will manage destruction of the 'msg' on errors */
431-
if (flux_future_aux_set (f, "msg", msgcpy, NULL) < 0) {
415+
if (flux_future_aux_set (f,
416+
"msg",
417+
(void *)flux_msg_incref (msg),
418+
(flux_free_f)flux_msg_decref) < 0) {
432419
flux_log_error (ctx->h, "%s: flux_future_aux_set", __FUNCTION__);
420+
flux_msg_decref (msg);
433421
goto error;
434422
}
435423

@@ -439,7 +427,6 @@ static int getroot_request_send (struct kvs_ctx *ctx,
439427
return 0;
440428
error:
441429
saved_errno = errno;
442-
flux_msg_destroy (msgcpy);
443430
flux_future_destroy (f);
444431
errno = saved_errno;
445432
return -1;
@@ -449,7 +436,6 @@ static struct kvsroot *getroot (struct kvs_ctx *ctx,
449436
const char *ns,
450437
flux_msg_handler_t *mh,
451438
const flux_msg_t *msg,
452-
lookup_t *lh,
453439
bool *stall)
454440
{
455441
struct kvsroot *root;
@@ -462,7 +448,7 @@ static struct kvsroot *getroot (struct kvs_ctx *ctx,
462448
return NULL;
463449
}
464450
else {
465-
if (getroot_request_send (ctx, ns, mh, msg, lh) < 0) {
451+
if (getroot_request_send (ctx, ns, mh, msg) < 0) {
466452
flux_log_error (ctx->h, "getroot_request_send");
467453
return NULL;
468454
}
@@ -738,26 +724,19 @@ static int content_store_request_send (struct kvs_ctx *ctx,
738724
int len)
739725
{
740726
flux_future_t *f;
741-
int saved_errno, rc = -1;
742727

743728
if (!(f = content_store (ctx->h, data, len, 0)))
729+
return -1;
730+
if (flux_future_aux_set (f, "cache_blobref", (void *)blobref, NULL) < 0)
744731
goto error;
745-
if (flux_future_aux_set (f, "cache_blobref", (void *)blobref, NULL) < 0) {
746-
saved_errno = errno;
747-
flux_future_destroy (f);
748-
errno = saved_errno;
749-
goto error;
750-
}
751-
if (flux_future_then (f, -1., content_store_completion, ctx) < 0) {
752-
saved_errno = errno;
753-
flux_future_destroy (f);
754-
errno = saved_errno;
732+
if (flux_future_then (f, -1., content_store_completion, ctx) < 0)
755733
goto error;
756-
}
757734

758-
rc = 0;
735+
return 0;
736+
759737
error:
760-
return rc;
738+
flux_future_destroy (f);
739+
return -1;
761740
}
762741

763742
static int kvstxn_load_cb (kvstxn_t *kt, const char *ref, void *data)
@@ -826,14 +805,13 @@ static int setroot_event_send (struct kvs_ctx *ctx,
826805
{
827806
flux_msg_t *msg = NULL;
828807
char *setroot_topic = NULL;
829-
int saved_errno, rc = -1;
808+
int rc = -1;
830809

831810
assert (ctx->rank == 0);
832811

833812
if (asprintf (&setroot_topic,
834813
"kvs.namespace-%s-setroot",
835814
root->ns_name) < 0) {
836-
saved_errno = errno;
837815
flux_log_error (ctx->h, "%s: asprintf", __FUNCTION__);
838816
goto done;
839817
}
@@ -846,24 +824,17 @@ static int setroot_event_send (struct kvs_ctx *ctx,
846824
"names", names,
847825
"keys", keys,
848826
"owner", root->owner))) {
849-
saved_errno = errno;
850827
flux_log_error (ctx->h, "%s: flux_event_pack", __FUNCTION__);
851828
goto done;
852829
}
853-
if (flux_msg_set_private (msg) < 0) {
854-
saved_errno = errno;
830+
if (flux_msg_set_private (msg) < 0)
855831
goto done;
856-
}
857-
if (flux_send (ctx->h, msg, 0) < 0) {
858-
saved_errno = errno;
832+
if (flux_send (ctx->h, msg, 0) < 0)
859833
goto done;
860-
}
861834
rc = 0;
862835
done:
863-
free (setroot_topic);
836+
ERRNO_SAFE_WRAP (free, setroot_topic);
864837
flux_msg_destroy (msg);
865-
if (rc < 0)
866-
errno = saved_errno;
867838
return rc;
868839
}
869840

@@ -874,10 +845,9 @@ static int error_event_send (struct kvs_ctx *ctx,
874845
{
875846
flux_msg_t *msg = NULL;
876847
char *error_topic = NULL;
877-
int saved_errno, rc = -1;
848+
int rc = -1;
878849

879850
if (asprintf (&error_topic, "kvs.namespace-%s-error", ns) < 0) {
880-
saved_errno = errno;
881851
flux_log_error (ctx->h, "%s: asprintf", __FUNCTION__);
882852
goto done;
883853
}
@@ -887,24 +857,17 @@ static int error_event_send (struct kvs_ctx *ctx,
887857
"namespace", ns,
888858
"names", names,
889859
"errnum", errnum))) {
890-
saved_errno = errno;
891860
flux_log_error (ctx->h, "%s: flux_event_pack", __FUNCTION__);
892861
goto done;
893862
}
894-
if (flux_msg_set_private (msg) < 0) {
895-
saved_errno = errno;
863+
if (flux_msg_set_private (msg) < 0)
896864
goto done;
897-
}
898-
if (flux_send (ctx->h, msg, 0) < 0) {
899-
saved_errno = errno;
865+
if (flux_send (ctx->h, msg, 0) < 0)
900866
goto done;
901-
}
902867
rc = 0;
903868
done:
904-
free (error_topic);
869+
ERRNO_SAFE_WRAP (free, error_topic);
905870
flux_msg_destroy (msg);
906-
if (rc < 0)
907-
errno = saved_errno;
908871
return rc;
909872
}
910873

@@ -914,16 +877,15 @@ static int error_event_send_to_name (struct kvs_ctx *ctx,
914877
int errnum)
915878
{
916879
json_t *names = NULL;
917-
int rc = -1;
880+
int rc;
918881

919882
if (!(names = json_pack ("[ s ]", name))) {
920883
errno = ENOMEM;
921884
flux_log_error (ctx->h, "%s: json_pack", __FUNCTION__);
922-
goto done;
885+
return -1;
923886
}
924887

925888
rc = error_event_send (ctx, ns, names, errnum);
926-
done:
927889
json_decref (names);
928890
return rc;
929891
}
@@ -1438,25 +1400,32 @@ static lookup_t *lookup_common (flux_t *h,
14381400
ns = lookup_missing_namespace (lh);
14391401
assert (ns);
14401402

1441-
root = getroot (ctx, ns, mh, msg, lh, &stall);
1403+
root = getroot (ctx, ns, mh, msg, &stall);
14421404
assert (!root);
14431405

1444-
if (stall)
1406+
if (stall) {
1407+
if (flux_msg_aux_set (msg, "lookup_handle", lh, NULL) < 0) {
1408+
flux_log_error (ctx->h, "%s: flux_msg_aux_set", __FUNCTION__);
1409+
goto done;
1410+
}
14451411
goto stall;
1412+
}
14461413
goto done;
14471414
}
14481415
else if (lret == LOOKUP_PROCESS_LOAD_MISSING_REFS) {
14491416
struct kvs_cb_data cbd;
14501417

1451-
if (!(wait = wait_create_msg_handler (h, mh, msg, ctx, replay_cb)))
1418+
/* do not destroy lookup_handle on message destruction, we
1419+
* manage it in here */
1420+
if (flux_msg_aux_set (msg, "lookup_handle", lh, NULL) < 0) {
1421+
flux_log_error (ctx->h, "%s: flux_msg_aux_set", __FUNCTION__);
14521422
goto done;
1423+
}
14531424

1454-
if (wait_set_error_cb (wait, lookup_wait_error_cb, lh) < 0)
1425+
if (!(wait = wait_create_msg_handler (h, mh, msg, ctx, replay_cb)))
14551426
goto done;
14561427

1457-
/* do not destroy lookup_handle on message destruction, we
1458-
* manage it in here */
1459-
if (wait_msg_aux_set (wait, "lookup_handle", lh, NULL) < 0)
1428+
if (wait_set_error_cb (wait, lookup_wait_error_cb, lh) < 0)
14601429
goto done;
14611430

14621431
cbd.ctx = ctx;
@@ -1764,7 +1733,7 @@ static void commit_request_cb (flux_t *h,
17641733
goto error;
17651734
}
17661735

1767-
if (!(root = getroot (ctx, ns, mh, msg, NULL, &stall))) {
1736+
if (!(root = getroot (ctx, ns, mh, msg, &stall))) {
17681737
if (stall) {
17691738
request_tracking_add (ctx, msg);
17701739
return;
@@ -1852,7 +1821,7 @@ static void wait_version_request_cb (flux_t *h,
18521821
goto error;
18531822
}
18541823

1855-
if (!(root = getroot (ctx, ns, mh, msg, NULL, &stall))) {
1824+
if (!(root = getroot (ctx, ns, mh, msg, &stall))) {
18561825
if (stall) {
18571826
request_tracking_add (ctx, msg);
18581827
return;
@@ -1918,7 +1887,7 @@ static void getroot_request_cb (flux_t *h, flux_msg_handler_t *mh,
19181887
* first.
19191888
*/
19201889
bool stall = false;
1921-
if (!(root = getroot (ctx, ns, mh, msg, NULL, &stall))) {
1890+
if (!(root = getroot (ctx, ns, mh, msg, &stall))) {
19221891
if (stall) {
19231892
request_tracking_add (ctx, msg);
19241893
return;
@@ -2382,7 +2351,7 @@ static void start_root_remove (struct kvs_ctx *ctx, const char *ns)
23822351
static int namespace_remove (struct kvs_ctx *ctx, const char *ns)
23832352
{
23842353
flux_msg_t *msg = NULL;
2385-
int saved_errno, rc = -1;
2354+
int rc = -1;
23862355
char *topic = NULL;
23872356

23882357
/* Namespace doesn't exist or is already in process of being
@@ -2392,32 +2361,23 @@ static int namespace_remove (struct kvs_ctx *ctx, const char *ns)
23922361
goto done;
23932362
}
23942363

2395-
if (asprintf (&topic, "kvs.namespace-%s-removed", ns) < 0) {
2396-
saved_errno = errno;
2364+
if (asprintf (&topic, "kvs.namespace-%s-removed", ns) < 0)
23972365
goto cleanup;
2398-
}
23992366
if (!(msg = flux_event_pack (topic, "{ s:s }", "namespace", ns))) {
2400-
saved_errno = errno;
24012367
flux_log_error (ctx->h, "%s: flux_event_pack", __FUNCTION__);
24022368
goto cleanup;
24032369
}
2404-
if (flux_msg_set_private (msg) < 0) {
2405-
saved_errno = errno;
2370+
if (flux_msg_set_private (msg) < 0)
24062371
goto cleanup;
2407-
}
2408-
if (flux_send (ctx->h, msg, 0) < 0) {
2409-
saved_errno = errno;
2372+
if (flux_send (ctx->h, msg, 0) < 0)
24102373
goto cleanup;
2411-
}
24122374

24132375
start_root_remove (ctx, ns);
24142376
done:
24152377
rc = 0;
24162378
cleanup:
24172379
flux_msg_destroy (msg);
2418-
free (topic);
2419-
if (rc < 0)
2420-
errno = saved_errno;
2380+
ERRNO_SAFE_WRAP (free, topic);
24212381
return rc;
24222382
}
24232383

@@ -2543,7 +2503,7 @@ static void setroot_pause_request_cb (flux_t *h,
25432503
goto error;
25442504
}
25452505

2546-
if (!(root = getroot (ctx, ns, mh, msg, NULL, &stall))) {
2506+
if (!(root = getroot (ctx, ns, mh, msg, &stall))) {
25472507
if (stall)
25482508
return;
25492509
goto error;
@@ -2608,7 +2568,7 @@ static void setroot_unpause_request_cb (flux_t *h,
26082568
goto error;
26092569
}
26102570

2611-
if (!(root = getroot (ctx, ns, mh, msg, NULL, &stall))) {
2571+
if (!(root = getroot (ctx, ns, mh, msg, &stall))) {
26122572
if (stall)
26132573
return;
26142574
goto error;
@@ -2947,12 +2907,11 @@ static int store_initial_rootdir (struct kvs_ctx *ctx, char *ref, int ref_len)
29472907
saved_errno = errno;
29482908
ret = cache_remove_entry (ctx->cache, ref);
29492909
assert (ret == 1);
2910+
errno = saved_errno;
29502911
error:
2951-
saved_errno = errno;
2952-
free (data);
2912+
ERRNO_SAFE_WRAP (free, data);
29532913
flux_future_destroy (f);
2954-
json_decref (rootdir);
2955-
errno = saved_errno;
2914+
ERRNO_SAFE_WRAP (json_decref, rootdir);
29562915
return -1;
29572916
}
29582917

0 commit comments

Comments
 (0)