Merge pull request #6622 from chu11/issue6608_kvs_dropcache_remove

kvs: remove dropcache

Author: mergify[bot]

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
 ----
 

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= \
     "

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
 

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;

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
  */

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

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",

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

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 <dir>
-     */
-    if (!(txn = flux_kvs_txn_create ()))
-        log_err_exit ("flux_kvs_txn_create");
-    if (flux_kvs_txn_mkdir (txn, 0, dir) < 0)
-        log_err_exit ("flux_kvs_txn_mkdir");
-    if (!(f = flux_kvs_commit (h, NULL, 0, txn)))
-        log_err_exit ("flux_kvs_commit");
-    if (flux_future_get (f, NULL) < 0)
-        log_err_exit ("flux_future_get");
-    flux_future_destroy (f);
-    flux_kvs_txn_destroy (txn);
-
-    /* Expire internal kvs cache
-     */
-    if (flux_kvs_dropcache (h) < 0)
-        log_err_exit ("flux_kvs_dropcache");
-
     /* Commit the following:
      * put <dir>.a
      * unlink <dir>

t/t1000-kvs.t

@@ -1164,17 +1164,6 @@ test_expect_success 'kvs: --sequence on write ops works' '
         test $VER -eq $SEQ
 '
 
-#
-# dropcache tests
-#
-
-test_expect_success 'kvs: dropcache works' '
-	flux kvs dropcache
-'
-test_expect_success 'kvs: dropcache --all works' '
-	flux kvs dropcache --all
-'
-
 #
 # version/wait tests
 #