Skip to content

Commit

Permalink
kvs-watch: avoid error response on failed rpc
Browse files Browse the repository at this point in the history
Problem: In the function handle_lookup_response(), if a call to
flux_respond_pack() fails, an additional call to flux_respond_error()
can be made in the "goto error".  This is not common usage in flux-core.
If flux_respond_pack() fails, we simply log an error and avoid a second
attempt to send a response.

In handle_lookup_response() do not automatically call flux_respond_error()
on an error.  In called function, call flux_respond_error() if necessary.
  • Loading branch information
chu11 committed Nov 22, 2024
1 parent fcfcbdc commit 0047178
Showing 1 changed file with 24 additions and 9 deletions.
33 changes: 24 additions & 9 deletions src/modules/kvs-watch/kvs-watch.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ static int handle_initial_response (flux_t *h,
NULL,
&w->append_offset) < 0) {
flux_log_error (h, "%s: treeobj_decode_val", __FUNCTION__);
return -1;
goto error_respond;
}
}

Expand All @@ -266,6 +266,13 @@ static int handle_initial_response (flux_t *h,
w->initial_rootseq = root_seq;
w->responded = true;
return 0;

error_respond:
if (!w->mute) {
if (flux_respond_error (h, w->request, errno, NULL) < 0)
flux_log_error (h, "%s: flux_respond_error", __FUNCTION__);
}
return -1;
}

static int handle_compare_response (flux_t *h,
Expand Down Expand Up @@ -319,7 +326,7 @@ static int handle_append_response (flux_t *h,
NULL,
&w->append_offset) < 0) {
flux_log_error (h, "%s: treeobj_decode_val", __FUNCTION__);
return -1;
goto error_respond;
}

if (flux_respond_pack (h, w->request, "{ s:O }", "val", val) < 0) {
Expand All @@ -340,7 +347,7 @@ static int handle_append_response (flux_t *h,
&new_data,
&new_offset) < 0) {
flux_log_error (h, "%s: treeobj_decode_val", __FUNCTION__);
return -1;
goto error_respond;
}

/* check length to determine if append actually happened, note
Expand All @@ -353,13 +360,13 @@ static int handle_append_response (flux_t *h,
if (new_offset < w->append_offset) {
free (new_data);
errno = EINVAL;
return -1;
goto error_respond;
}

if (!(new_val = treeobj_create_val (new_data + w->append_offset,
new_offset - w->append_offset))) {
free (new_data);
return -1;
goto error_respond;
}

free (new_data);
Expand All @@ -375,6 +382,13 @@ static int handle_append_response (flux_t *h,
}

return 0;

error_respond:
if (!w->mute) {
if (flux_respond_error (h, w->request, errno, NULL) < 0)
flux_log_error (h, "%s: flux_respond_error", __FUNCTION__);
}
return -1;
}

static int handle_normal_response (flux_t *h,
Expand Down Expand Up @@ -449,7 +463,7 @@ static void handle_lookup_response (flux_future_t *f,
}

if (handle_initial_response (h, w, val, root_seq) < 0)
goto error;
goto finished;
}
else {
/* First check for ENOENT */
Expand All @@ -473,15 +487,15 @@ static void handle_lookup_response (flux_future_t *f,
if ((w->flags & FLUX_KVS_WATCH_FULL)
|| (w->flags & FLUX_KVS_WATCH_UNIQ)) {
if (handle_compare_response (h, w, val) < 0)
goto error;
goto finished;
}
else if (w->flags & FLUX_KVS_WATCH_APPEND) {
if (handle_append_response (h, w, val) < 0)
goto error;
goto finished;
}
else {
if (handle_normal_response (h, w, val) < 0)
goto error;
goto finished;
}
}
}
Expand All @@ -491,6 +505,7 @@ static void handle_lookup_response (flux_future_t *f,
if (flux_respond_error (h, w->request, errno, NULL) < 0)
flux_log_error (h, "%s: flux_respond_error", __FUNCTION__);
}
finished:
w->finished = true;
}

Expand Down

0 comments on commit 0047178

Please sign in to comment.