diff --git a/doc/man1/flux-kvs.rst b/doc/man1/flux-kvs.rst index 1c657ed85053..de972011bcff 100644 --- a/doc/man1/flux-kvs.rst +++ b/doc/man1/flux-kvs.rst @@ -14,7 +14,6 @@ SYNOPSIS | **flux** **kvs** **link** *target* *linkname* | **flux** **kvs** **readlink** *key...* | **flux** **kvs** **mkdir** *key...* -| **flux** **kvs** **dropcache** | **flux** **kvs** **copy** *source* *destination* | **flux** **kvs** **move** *source* *destination* @@ -361,18 +360,6 @@ Create an empty directory. If *key* exists, it is overwritten. After the commit has completed, display the new root sequence number or "version". -dropcache ---------- - -.. program:: flux kvs dropcache - -Tell the local KVS to drop any cache it is holding. - -.. option:: -a, --all - - Publish an event across the Flux instance instructing the KVS module on - all ranks to drop their caches. - copy ---- diff --git a/etc/completions/flux.pre b/etc/completions/flux.pre index f215ee28fc27..f90dbb97eb86 100644 --- a/etc/completions/flux.pre +++ b/etc/completions/flux.pre @@ -1027,7 +1027,6 @@ _flux_kvs() mkdir copy move - dropcache version wait getroot @@ -1106,9 +1105,6 @@ _flux_kvs() move_OPTS="\ ${copy_OPTS} " - dropcache_OPTS="\ - -a --all - " version_OPTS="\ -N --namespace= \ " diff --git a/src/bindings/python/flux/kvs.py b/src/bindings/python/flux/kvs.py index 20eb79b5ed7d..7950fdae5016 100644 --- a/src/bindings/python/flux/kvs.py +++ b/src/bindings/python/flux/kvs.py @@ -366,17 +366,6 @@ def namespace_list(flux_handle): return nslist -def dropcache(flux_handle): - """Drop KVS cache entries - - Inform KVS module to drop cache entries without a reference. - - Args: - flux_handle: A Flux handle obtained from flux.Flux() - """ - RAW.flux_kvs_dropcache(flux_handle) - - class KVSTxn: """KVS Transaction Object diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index c3b88304d9b7..d64de132ae58 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -42,7 +42,6 @@ int cmd_readlink (optparse_t *p, int argc, char **argv); int cmd_mkdir (optparse_t *p, int argc, char **argv); int cmd_version (optparse_t *p, int argc, char **argv); int cmd_wait (optparse_t *p, int argc, char **argv); -int cmd_dropcache (optparse_t *p, int argc, char **argv); int cmd_copy (optparse_t *p, int argc, char **argv); int cmd_move (optparse_t *p, int argc, char **argv); int cmd_dir (optparse_t *p, int argc, char **argv); @@ -184,13 +183,6 @@ static struct optparse_option ls_opts[] = { OPTPARSE_TABLE_END }; -static struct optparse_option dropcache_opts[] = { - { .name = "all", .key = 'a', .has_arg = 0, - .usage = "Drop KVS across all ranks", - }, - OPTPARSE_TABLE_END -}; - static struct optparse_option unlink_opts[] = { { .name = "namespace", .key = 'N', .has_arg = 1, .usage = "Specify KVS namespace to use.", @@ -360,13 +352,6 @@ static struct optparse_subcommand subcommands[] = { 0, copy_opts }, - { "dropcache", - "[--all]", - "Tell KVS to drop its cache", - cmd_dropcache, - 0, - dropcache_opts - }, { "version", "[-N ns]", "Display current KVS version", @@ -1213,27 +1198,6 @@ int cmd_wait (optparse_t *p, int argc, char **argv) return (0); } -int cmd_dropcache (optparse_t *p, int argc, char **argv) -{ - flux_t *h; - - if (!(h = flux_open (NULL, 0))) - log_err_exit ("flux_open"); - - if (optparse_hasopt (p, "all")) { - flux_msg_t *msg = flux_event_encode ("kvs.dropcache", NULL); - if (!msg || flux_send (h, msg, 0) < 0) - log_err_exit ("flux_send"); - flux_msg_destroy (msg); - } - else { - if (flux_kvs_dropcache (h) < 0) - log_err_exit ("flux_kvs_dropcache"); - } - flux_close (h); - return (0); -} - static char *process_key (const char *key) { char *nkey; diff --git a/src/common/libkvs/kvs.c b/src/common/libkvs/kvs.c index 53564240b9f2..113e932c23fc 100644 --- a/src/common/libkvs/kvs.c +++ b/src/common/libkvs/kvs.c @@ -162,21 +162,6 @@ int flux_kvs_wait_version (flux_t *h, const char *ns, int version) return ret; } -int flux_kvs_dropcache (flux_t *h) -{ - flux_future_t *f; - int rc = -1; - - if (!(f = flux_rpc (h, "kvs.dropcache", NULL, FLUX_NODEID_ANY, 0))) - goto done; - if (flux_future_get (f, NULL) < 0) - goto done; - rc = 0; -done: - flux_future_destroy (f); - return rc; -} - /* * vi:tabstop=4 shiftwidth=4 expandtab */ diff --git a/src/common/libkvs/kvs.h b/src/common/libkvs/kvs.h index bec5a4d47f4b..2cf93988edae 100644 --- a/src/common/libkvs/kvs.h +++ b/src/common/libkvs/kvs.h @@ -66,12 +66,6 @@ flux_future_t *flux_kvs_namespace_remove (flux_t *h, const char *ns); int flux_kvs_get_version (flux_t *h, const char *ns, int *versionp); int flux_kvs_wait_version (flux_t *h, const char *ns, int version); -/* Garbage collect the cache. Drop all data that doesn't have a - * reference in the namespace. - * Returns -1 on error (errno set), 0 on success. - */ -int flux_kvs_dropcache (flux_t *h); - #ifdef __cplusplus } #endif diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 1381b5052712..7d212cf55292 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -222,15 +222,6 @@ static int event_subscribe (struct kvs_ctx *ctx, const char *ns) */ if (!(ctx->events_init)) { - - /* These belong to all namespaces, subscribe once the first - * time we init a namespace */ - - if (flux_event_subscribe (ctx->h, "kvs.dropcache") < 0) { - flux_log_error (ctx->h, "flux_event_subscribe"); - goto cleanup; - } - /* On rank 0, we need to listen for all of these namespace * events, all of the time. So subscribe to them just once on * rank 0. */ @@ -1171,62 +1162,6 @@ static void transaction_check_cb (flux_reactor_t *r, * rpc/event callbacks */ -static void dropcache_request_cb (flux_t *h, flux_msg_handler_t *mh, - const flux_msg_t *msg, void *arg) -{ - struct kvs_ctx *ctx = arg; - int size, expcount = 0; - - /* irrelevant if root not initialized, drop cache entries */ - - if (flux_request_decode (msg, NULL, NULL) < 0) - goto error; - size = cache_count_entries (ctx->cache); - if ((expcount = cache_expire_entries (ctx->cache, 0)) < 0) { - flux_log_error (ctx->h, "%s: cache_expire_entries", __FUNCTION__); - goto error; - } - else { - flux_log (h, - LOG_ALERT, - "dropped %d of %d cache entries", - expcount, - size); - } - if (flux_respond (h, msg, NULL) < 0) - flux_log_error (h, "%s: flux_respond", __FUNCTION__); - return; -error: - if (flux_respond_error (h, msg, errno, NULL) < 0) - flux_log_error (h, "%s: flux_respond_error", __FUNCTION__); -} - -static void dropcache_event_cb (flux_t *h, - flux_msg_handler_t *mh, - const flux_msg_t *msg, - void *arg) -{ - struct kvs_ctx *ctx = arg; - int size, expcount = 0; - - /* irrelevant if root not initialized, drop cache entries */ - - if (flux_event_decode (msg, NULL, NULL) < 0) { - flux_log_error (ctx->h, "%s: flux_event_decode", __FUNCTION__); - return; - } - size = cache_count_entries (ctx->cache); - if ((expcount = cache_expire_entries (ctx->cache, 0)) < 0) - flux_log_error (ctx->h, "%s: cache_expire_entries", __FUNCTION__); - else { - flux_log (h, - LOG_ALERT, - "dropped %d of %d cache entries", - expcount, - size); - } -} - static int heartbeat_root_cb (struct kvsroot *root, void *arg) { struct kvs_ctx *ctx = arg; @@ -2600,18 +2535,6 @@ static const struct flux_msg_handler_spec htab[] = { getroot_request_cb, FLUX_ROLE_USER }, - { - FLUX_MSGTYPE_REQUEST, - "kvs.dropcache", - dropcache_request_cb, - 0 - }, - { - FLUX_MSGTYPE_EVENT, - "kvs.dropcache", - dropcache_event_cb, - 0 - }, { FLUX_MSGTYPE_REQUEST, "kvs.disconnect", diff --git a/t/issues/t1760-kvs-use-after-free.sh b/t/issues/t1760-kvs-use-after-free.sh index 7a371b5b7339..75bb095c4f73 100755 --- a/t/issues/t1760-kvs-use-after-free.sh +++ b/t/issues/t1760-kvs-use-after-free.sh @@ -1,6 +1,34 @@ #!/bin/sh -e # dropcache, do a put that misses a references, unlink a dir, the reference # should not be missing. +# +# N.B. the reason this test is split across two flux starts is we need the +# internal KVS cache to be empty -TEST=issue1760 -${FLUX_BUILD_DIR}/t/kvs/issue1760 a +cat <<-EOF >t1760setup.sh +#!/bin/sh -e + +flux kvs mkdir foo + +EOF + +cat <<-EOF >t1760test.sh +#!/bin/sh -e + +${FLUX_BUILD_DIR}/t/kvs/issue1760 foo + +EOF + +chmod +x t1760setup.sh +chmod +x t1760test.sh + +STATEDIR=issue1760-statedir +mkdir issue1760-statedir + +flux start -s 1 \ + --setattr=statedir=${STATEDIR} \ + ./t1760setup.sh + +flux start -s 1 \ + --setattr=statedir=${STATEDIR} \ + ./t1760test.sh diff --git a/t/kvs/issue1760.c b/t/kvs/issue1760.c index ac9dec326bb5..3b88f666cfb0 100644 --- a/t/kvs/issue1760.c +++ b/t/kvs/issue1760.c @@ -8,7 +8,13 @@ * SPDX-License-Identifier: LGPL-3.0 \************************************************************/ -/* issue1760.c - make kvs module sad */ +/* issue1760.c - make kvs module sad + * + * - it is assumed the directory passed in under argv[1] has already + * been created. The command line `flux kvs` command does not allow + * a put & unlink to be done under a single transaction, thus the need + * for this utility test. + */ /* Failure mode 1: ./issue1760 a @@ -52,24 +58,6 @@ int main (int argc, char *argv[]) if (!(h = flux_open (NULL, 0))) log_err_exit ("flux_open"); - /* Mkdir