Upcall: Cleanup expired client entries

To cleanup expired client entries (with access_time > 2*CACHE_INVALIDATION_TIMEOUT),
have
        * defined a global list to contain all the upcall_inode_ctx allocated
        * Every time a upcall_inode_ctx is allocated, it is added to the global list
        * during inode_forget, that upcall_inode_ctx is marked for destroy
        * created a reaper thread which scans through that list
                * cleans up expired client entries
                * frees the inode_ctx with destroy_mode set.

Note: This reaper thread is initialized only when features.cache_invalidation option
is enabled.

Change-Id: Iea2a63eb31b8e08d5709e7e090cf26fd13d01265
BUG: 1200267
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
Reviewed-on: http://review.gluster.org/10342
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>

4 files changed, 195 insertions, 18 deletions

diff --git a/xlators/features/upcall/src/upcall-internal.c b/xlators/features/upcall/src/upcall-internal.c
index a2c33cb2ca7..19d19289624 100644
--- a/xlators/features/upcall/src/upcall-internal.c
+++ b/xlators/features/upcall/src/upcall-internal.c
@@ -181,9 +181,13 @@ int
 __upcall_inode_ctx_set (inode_t *inode, xlator_t *this)
 {
         upcall_inode_ctx_t *inode_ctx   = NULL;
+        upcall_private_t   *priv        = NULL;
         int                ret          = -1;
         uint64_t           ctx          = 0;
 
+        priv = this->private;
+        GF_ASSERT(priv);
+
         ret = __inode_ctx_get (inode, this, &ctx);
 
         if (!ret)
@@ -198,12 +202,22 @@ __upcall_inode_ctx_set (inode_t *inode, xlator_t *this)
         }
 
         pthread_mutex_init (&inode_ctx->client_list_lock, NULL);
+        INIT_LIST_HEAD (&inode_ctx->inode_ctx_list);
         INIT_LIST_HEAD (&inode_ctx->client_list);
+        inode_ctx->destroy = 0;
 
         ret = __inode_ctx_set (inode, this, (uint64_t *) inode_ctx);
         if (ret)
                 gf_log (this->name, GF_LOG_DEBUG,
                         "failed to set inode ctx (%p)", inode);
+
+        /* add this inode_ctx to the global list */
+        LOCK (&priv->inode_ctx_lk);
+        {
+                list_add_tail (&inode_ctx->inode_ctx_list,
+                               &priv->inode_ctx_list);
+        }
+        UNLOCK (&priv->inode_ctx_lk);
 out:
         return ret;
 }
@@ -248,6 +262,51 @@ upcall_inode_ctx_get (inode_t *inode, xlator_t *this)
 }
 
 int
+upcall_cleanup_expired_clients (xlator_t *this,
+                                upcall_inode_ctx_t *up_inode_ctx) {
+
+        upcall_client_t *up_client      = NULL;
+        upcall_client_t *tmp            = NULL;
+        int             ret             = -1;
+        time_t          timeout         = 0;
+        time_t          t_expired       = 0;
+
+        timeout = get_cache_invalidation_timeout(this);
+
+        pthread_mutex_lock (&up_inode_ctx->client_list_lock);
+        {
+                list_for_each_entry_safe (up_client,
+                                          tmp,
+                                          &up_inode_ctx->client_list,
+                                          client_list) {
+                        t_expired = time(NULL) -
+                                         up_client->access_time;
+
+                        if (t_expired > (2*timeout)) {
+                                ret =
+                                  __upcall_cleanup_client_entry (up_client);
+
+                                if (ret) {
+                                        gf_msg ("upcall", GF_LOG_WARNING, 0,
+                                                UPCALL_MSG_INTERNAL_ERROR,
+                                                "Client entry cleanup failed (%p)",
+                                                up_client);
+                                        goto out;
+                                }
+                                gf_log (THIS->name, GF_LOG_TRACE,
+                                        "Cleaned up client_entry(%s) of",
+                                        up_client->client_uid);
+                        }
+                }
+        }
+        pthread_mutex_unlock (&up_inode_ctx->client_list_lock);
+
+        ret = 0;
+out:
+        return ret;
+}
+
+int
 __upcall_cleanup_client_entry (upcall_client_t *up_client)
 {
         list_del_init (&up_client->client_list);
@@ -285,31 +344,31 @@ upcall_cleanup_inode_ctx (xlator_t *this, inode_t *inode)
         uint64_t           ctx          = 0;
         upcall_inode_ctx_t *inode_ctx   = NULL;
         int                ret          = 0;
+        upcall_private_t   *priv        = NULL;
+
+        priv = this->private;
+        GF_ASSERT(priv);
 
-        ret = inode_ctx_get (inode, this, &ctx);
+        ret = inode_ctx_del (inode, this, &ctx);
 
         if (ret < 0) {
-                gf_log (THIS->name, GF_LOG_TRACE,
-                        "Failed to get upcall_inode_ctx (%p)",
+                gf_msg ("upcall", GF_LOG_WARNING, 0,
+                        UPCALL_MSG_INTERNAL_ERROR,
+                        "Failed to del upcall_inode_ctx (%p)",
                         inode);
                 goto out;
         }
 
-        /* Invalidate all the upcall cache entries */
-        upcall_cache_forget (this, inode, inode_ctx);
-
-        /* Set inode context to NULL */
-        ret = __inode_ctx_set (inode, this, NULL);
-
-        if (!ret) {
-                gf_log (this->name, GF_LOG_WARNING,
-                        "_inode_ctx_set to NULL failed (%p)",
-                        inode);
-        }
         inode_ctx = (upcall_inode_ctx_t *)(long) ctx;
 
         if (inode_ctx) {
-                /* do we really need lock? */
+
+                /* Invalidate all the upcall cache entries */
+                upcall_cache_forget (this, inode, inode_ctx);
+
+                /* do we really need lock? yes now reaper thread
+                 * may also be trying to cleanup the client entries.
+                 */
                 pthread_mutex_lock (&inode_ctx->client_list_lock);
                 {
                         if (!list_empty (&inode_ctx->client_list)) {
@@ -320,13 +379,77 @@ upcall_cleanup_inode_ctx (xlator_t *this, inode_t *inode)
 
                 pthread_mutex_destroy (&inode_ctx->client_list_lock);
 
-                GF_FREE (inode_ctx);
+                /* Mark the inode_ctx to be destroyed */
+                inode_ctx->destroy = 1;
         }
 
 out:
         return ret;
 }
 
+/*
+ * Traverse through the list of upcall_inode_ctx(s),
+ * cleanup the expired client entries and destroy the ctx
+ * which is no longer valid and has destroy bit set.
+ */
+void *
+upcall_reaper_thread (void *data)
+{
+        upcall_private_t *priv          = NULL;
+        upcall_inode_ctx_t *inode_ctx   = NULL;
+        upcall_inode_ctx_t *tmp         = NULL;
+        xlator_t           *this        = NULL;
+
+        this = (xlator_t *)data;
+        GF_ASSERT (this);
+
+        priv = this->private;
+        GF_ASSERT (priv);
+
+
+        while (!priv->fini) {
+                list_for_each_entry_safe (inode_ctx, tmp,
+                                          &priv->inode_ctx_list,
+                                          inode_ctx_list) {
+
+                        /* cleanup expired clients */
+                        upcall_cleanup_expired_clients (this, inode_ctx);
+
+                        if (!inode_ctx->destroy) {
+                                continue;
+                        }
+
+                        LOCK (&priv->inode_ctx_lk);
+                        {
+                                /* client list would have been cleaned up*/
+                                list_del_init (&inode_ctx->inode_ctx_list);
+                                GF_FREE (inode_ctx);
+                        }
+                        UNLOCK (&priv->inode_ctx_lk);
+                }
+        }
+
+        return NULL;
+}
+
+/*
+ * Initialize upcall reaper thread.
+ */
+int
+upcall_reaper_thread_init (xlator_t *this)
+{
+        upcall_private_t *priv  = NULL;
+        int              ret    = -1;
+
+        priv = this->private;
+        GF_ASSERT (priv);
+
+        ret = pthread_create (&priv->reaper_thr, NULL,
+                              upcall_reaper_thread, this);
+
+        return ret;
+}
+
 void
 upcall_cache_invalidate_dir (call_frame_t *frame, xlator_t *this,
                              client_t *client, inode_t *inode, uint32_t flags)
@@ -374,7 +497,8 @@ upcall_cache_invalidate (call_frame_t *frame, xlator_t *this, client_t *client,
                 up_inode_ctx = upcall_inode_ctx_get (inode, this);
 
         if (!up_inode_ctx) {
-                gf_log (this->name, GF_LOG_WARNING,
+                gf_msg ("upcall", GF_LOG_WARNING, 0,
+                        UPCALL_MSG_INTERNAL_ERROR,
                         "upcall_inode_ctx_get failed (%p)",
                         inode);
                 return;
diff --git a/xlators/features/upcall/src/upcall-messages.h b/xlators/features/upcall/src/upcall-messages.h
index c8483f16528..7b004d678c3 100644
--- a/xlators/features/upcall/src/upcall-messages.h
+++ b/xlators/features/upcall/src/upcall-messages.h
@@ -56,6 +56,7 @@
  * @recommendedaction None
  */
 #define UPCALL_MSG_NO_MEMORY             (GLFS_COMP_BASE_UPCALL + 1)
+#define UPCALL_MSG_INTERNAL_ERROR        (GLFS_COMP_BASE_UPCALL + 2)
 
 #define glfs_msg_end_x GLFS_MSGID_END, "Invalid: End of messages"
 
diff --git a/xlators/features/upcall/src/upcall.c b/xlators/features/upcall/src/upcall.c
index ad86567aa7c..c68c0258fb8 100644
--- a/xlators/features/upcall/src/upcall.c
+++ b/xlators/features/upcall/src/upcall.c
@@ -1615,6 +1615,20 @@ reconfigure (xlator_t *this, dict_t *options)
                           options, int32, out);
 
         ret = 0;
+
+        if (priv->cache_invalidation_enabled &&
+            !priv->reaper_init_done) {
+                ret = upcall_reaper_thread_init (this);
+
+                if (ret) {
+                        gf_msg ("upcall", GF_LOG_WARNING, 0,
+                                UPCALL_MSG_INTERNAL_ERROR,
+                                "reaper_thread creation failed (%s)."
+                                " Disabling cache_invalidation",
+                                strerror(errno));
+                }
+        }
+
 out:
         return ret;
 }
@@ -1639,10 +1653,27 @@ init (xlator_t *this)
         GF_OPTION_INIT ("cache-invalidation-timeout",
                         priv->cache_invalidation_timeout, int32, out);
 
+        LOCK_INIT (&priv->inode_ctx_lk);
+        INIT_LIST_HEAD (&priv->inode_ctx_list);
+
         this->private = priv;
+        priv->fini = 0;
+        priv->reaper_init_done = 0;
+
         this->local_pool = mem_pool_new (upcall_local_t, 512);
         ret = 0;
 
+        if (priv->cache_invalidation_enabled) {
+                ret = upcall_reaper_thread_init (this);
+
+                if (ret) {
+                        gf_msg ("upcall", GF_LOG_WARNING, 0,
+                                UPCALL_MSG_INTERNAL_ERROR,
+                                "reaper_thread creation failed (%s)."
+                                " Disabling cache_invalidation",
+                                strerror(errno));
+                }
+        }
 out:
         if (ret) {
                 GF_FREE (priv);
@@ -1662,6 +1693,15 @@ fini (xlator_t *this)
         }
         this->private = NULL;
 
+        priv->fini = 1;
+
+        pthread_join (priv->reaper_thr, NULL);
+
+        LOCK_DESTROY (&priv->inode_ctx_lk);
+
+        /* Do we need to cleanup the inode_ctxs? IMO not required
+         * as inode_forget would have been done on all the inodes
+         * before calling xlator_fini */
         GF_FREE (priv);
 
         return 0;
diff --git a/xlators/features/upcall/src/upcall.h b/xlators/features/upcall/src/upcall.h
index 7e15f6c97cd..41895786382 100644
--- a/xlators/features/upcall/src/upcall.h
+++ b/xlators/features/upcall/src/upcall.h
@@ -56,6 +56,11 @@
 struct _upcall_private_t {
         gf_boolean_t     cache_invalidation_enabled;
         int32_t          cache_invalidation_timeout;
+        struct list_head inode_ctx_list;
+        gf_lock_t        inode_ctx_lk;
+        int32_t          reaper_init_done;
+        pthread_t        reaper_thr;
+        int32_t          fini;
 };
 typedef struct _upcall_private_t upcall_private_t;
 
@@ -71,9 +76,11 @@ typedef struct _upcall_client_t upcall_client_t;
 
 /* Upcall entries are maintained in inode_ctx */
 struct _upcall_inode_ctx_t {
+        struct list_head inode_ctx_list;
         struct list_head client_list;
         pthread_mutex_t client_list_lock; /* mutex for clients list
-                                                of this upcall entry */
+                                             of this upcall entry */
+        int destroy;
 };
 typedef struct _upcall_inode_ctx_t upcall_inode_ctx_t;
 
@@ -113,6 +120,8 @@ upcall_client_t *__get_upcall_client (call_frame_t *frame, uuid_t gfid,
                                       client_t *client,
                                       upcall_inode_ctx_t *up_inode_ctx);
 int __upcall_cleanup_client_entry (upcall_client_t *up_client);
+int upcall_cleanup_expired_clients (xlator_t *this,
+                                    upcall_inode_ctx_t *up_inode_ctx);
 
 int __upcall_inode_ctx_set (inode_t *inode, xlator_t *this);
 upcall_inode_ctx_t *__upcall_inode_ctx_get (inode_t *inode, xlator_t *this);
@@ -121,6 +130,9 @@ int upcall_cleanup_inode_ctx (xlator_t *this, inode_t *inode);
 void upcall_cache_forget (xlator_t *this, inode_t *inode,
                           upcall_inode_ctx_t *up_inode_ctx);
 
+void *upcall_reaper_thread (void *data);
+int upcall_reaper_thread_init (xlator_t *this);
+
 /* Xlator options */
 gf_boolean_t is_upcall_enabled(xlator_t *this);