17b94a
From cca418b78ec976aa69eacd56b0e6127ea7e3dd26 Mon Sep 17 00:00:00 2001
17b94a
From: Pranith Kumar K <pkarampu@redhat.com>
17b94a
Date: Thu, 4 Apr 2019 15:31:56 +0530
17b94a
Subject: [PATCH 095/124] cluster/afr: Remove local from owners_list on failure
17b94a
 of lock-acquisition
17b94a
17b94a
	Backport of https://review.gluster.org/c/glusterfs/+/22515
17b94a
17b94a
When eager-lock lock acquisition fails because of say network failures, the
17b94a
local is not being removed from owners_list, this leads to accumulation of
17b94a
waiting frames and the application will hang because the waiting frames are
17b94a
under the assumption that another transaction is in the process of acquiring
17b94a
lock because owner-list is not empty. Handled this case as well in this patch.
17b94a
Added asserts to make it easier to find these problems in future.
17b94a
17b94a
Change-Id: I3101393265e9827755725b1f2d94a93d8709e923
17b94a
fixes: bz#1688395
17b94a
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
17b94a
Reviewed-on: https://code.engineering.redhat.com/gerrit/167859
17b94a
Tested-by: RHGS Build Bot <nigelb@redhat.com>
17b94a
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
17b94a
---
17b94a
 tests/bugs/replicate/bug-1696599-io-hang.t | 47 ++++++++++++++++++++++++++++++
17b94a
 xlators/cluster/afr/src/afr-common.c       |  8 ++---
17b94a
 xlators/cluster/afr/src/afr-lk-common.c    |  1 -
17b94a
 xlators/cluster/afr/src/afr-transaction.c  | 19 +++++-------
17b94a
 xlators/cluster/afr/src/afr.h              |  4 +--
17b94a
 5 files changed, 61 insertions(+), 18 deletions(-)
17b94a
 create mode 100755 tests/bugs/replicate/bug-1696599-io-hang.t
17b94a
17b94a
diff --git a/tests/bugs/replicate/bug-1696599-io-hang.t b/tests/bugs/replicate/bug-1696599-io-hang.t
17b94a
new file mode 100755
17b94a
index 0000000..869cdb9
17b94a
--- /dev/null
17b94a
+++ b/tests/bugs/replicate/bug-1696599-io-hang.t
17b94a
@@ -0,0 +1,47 @@
17b94a
+#!/bin/bash
17b94a
+
17b94a
+. $(dirname $0)/../../include.rc
17b94a
+. $(dirname $0)/../../volume.rc
17b94a
+. $(dirname $0)/../../fileio.rc
17b94a
+
17b94a
+#Tests that local structures in afr are removed from granted/blocked list of
17b94a
+#locks when inodelk fails on all bricks
17b94a
+
17b94a
+cleanup;
17b94a
+
17b94a
+TEST glusterd
17b94a
+TEST pidof glusterd
17b94a
+
17b94a
+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{1..3}
17b94a
+TEST $CLI volume set $V0 performance.quick-read off
17b94a
+TEST $CLI volume set $V0 performance.write-behind off
17b94a
+TEST $CLI volume set $V0 performance.io-cache off
17b94a
+TEST $CLI volume set $V0 performance.stat-prefetch off
17b94a
+TEST $CLI volume set $V0 performance.client-io-threads off
17b94a
+TEST $CLI volume set $V0 delay-gen locks
17b94a
+TEST $CLI volume set $V0 delay-gen.delay-duration 5000000
17b94a
+TEST $CLI volume set $V0 delay-gen.delay-percentage 100
17b94a
+TEST $CLI volume set $V0 delay-gen.enable finodelk
17b94a
+
17b94a
+TEST $CLI volume start $V0
17b94a
+EXPECT 'Started' volinfo_field $V0 'Status'
17b94a
+
17b94a
+TEST $GFS -s $H0 --volfile-id $V0 $M0
17b94a
+TEST touch $M0/file
17b94a
+#Trigger write and stop bricks so inodelks fail on all bricks leading to
17b94a
+#lock failure condition
17b94a
+echo abc >> $M0/file &
17b94a
+
17b94a
+TEST $CLI volume stop $V0
17b94a
+TEST $CLI volume reset $V0 delay-gen
17b94a
+wait
17b94a
+TEST $CLI volume start $V0
17b94a
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 0
17b94a
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 1
17b94a
+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_meta $M0 $V0-replicate-0 2
17b94a
+#Test that only one write succeeded, this tests that delay-gen worked as
17b94a
+#expected
17b94a
+echo abc >> $M0/file
17b94a
+EXPECT "abc" cat $M0/file
17b94a
+
17b94a
+cleanup;
17b94a
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
17b94a
index 45b96e3..47a5d3a 100644
17b94a
--- a/xlators/cluster/afr/src/afr-common.c
17b94a
+++ b/xlators/cluster/afr/src/afr-common.c
17b94a
@@ -5763,6 +5763,10 @@ afr_transaction_local_init(afr_local_t *local, xlator_t *this)
17b94a
     afr_private_t *priv = NULL;
17b94a
 
17b94a
     priv = this->private;
17b94a
+    INIT_LIST_HEAD(&local->transaction.wait_list);
17b94a
+    INIT_LIST_HEAD(&local->transaction.owner_list);
17b94a
+    INIT_LIST_HEAD(&local->ta_waitq);
17b94a
+    INIT_LIST_HEAD(&local->ta_onwireq);
17b94a
     ret = afr_internal_lock_init(&local->internal_lock, priv->child_count);
17b94a
     if (ret < 0)
17b94a
         goto out;
17b94a
@@ -5800,10 +5804,6 @@ afr_transaction_local_init(afr_local_t *local, xlator_t *this)
17b94a
         goto out;
17b94a
 
17b94a
     ret = 0;
17b94a
-    INIT_LIST_HEAD(&local->transaction.wait_list);
17b94a
-    INIT_LIST_HEAD(&local->transaction.owner_list);
17b94a
-    INIT_LIST_HEAD(&local->ta_waitq);
17b94a
-    INIT_LIST_HEAD(&local->ta_onwireq);
17b94a
 out:
17b94a
     return ret;
17b94a
 }
17b94a
diff --git a/xlators/cluster/afr/src/afr-lk-common.c b/xlators/cluster/afr/src/afr-lk-common.c
17b94a
index 4091671..bc8eabe 100644
17b94a
--- a/xlators/cluster/afr/src/afr-lk-common.c
17b94a
+++ b/xlators/cluster/afr/src/afr-lk-common.c
17b94a
@@ -397,7 +397,6 @@ afr_unlock_now(call_frame_t *frame, xlator_t *this)
17b94a
     int_lock->lk_call_count = call_count;
17b94a
 
17b94a
     if (!call_count) {
17b94a
-        GF_ASSERT(!local->transaction.do_eager_unlock);
17b94a
         gf_msg_trace(this->name, 0, "No internal locks unlocked");
17b94a
         int_lock->lock_cbk(frame, this);
17b94a
         goto out;
17b94a
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
17b94a
index 229820b..15f3a7e 100644
17b94a
--- a/xlators/cluster/afr/src/afr-transaction.c
17b94a
+++ b/xlators/cluster/afr/src/afr-transaction.c
17b94a
@@ -372,6 +372,8 @@ afr_transaction_done(call_frame_t *frame, xlator_t *this)
17b94a
     }
17b94a
     local->transaction.unwind(frame, this);
17b94a
 
17b94a
+    GF_ASSERT(list_empty(&local->transaction.owner_list));
17b94a
+    GF_ASSERT(list_empty(&local->transaction.wait_list));
17b94a
     AFR_STACK_DESTROY(frame);
17b94a
 
17b94a
     return 0;
17b94a
@@ -393,7 +395,7 @@ afr_lock_fail_shared(afr_local_t *local, struct list_head *list)
17b94a
 }
17b94a
 
17b94a
 static void
17b94a
-afr_handle_lock_acquire_failure(afr_local_t *local, gf_boolean_t locked)
17b94a
+afr_handle_lock_acquire_failure(afr_local_t *local)
17b94a
 {
17b94a
     struct list_head shared;
17b94a
     afr_lock_t *lock = NULL;
17b94a
@@ -414,13 +416,8 @@ afr_handle_lock_acquire_failure(afr_local_t *local, gf_boolean_t locked)
17b94a
     afr_lock_fail_shared(local, &shared);
17b94a
     local->transaction.do_eager_unlock = _gf_true;
17b94a
 out:
17b94a
-    if (locked) {
17b94a
-        local->internal_lock.lock_cbk = afr_transaction_done;
17b94a
-        afr_unlock(local->transaction.frame, local->transaction.frame->this);
17b94a
-    } else {
17b94a
-        afr_transaction_done(local->transaction.frame,
17b94a
-                             local->transaction.frame->this);
17b94a
-    }
17b94a
+    local->internal_lock.lock_cbk = afr_transaction_done;
17b94a
+    afr_unlock(local->transaction.frame, local->transaction.frame->this);
17b94a
 }
17b94a
 
17b94a
 call_frame_t *
17b94a
@@ -619,7 +616,7 @@ afr_transaction_perform_fop(call_frame_t *frame, xlator_t *this)
17b94a
     failure_count = AFR_COUNT(local->transaction.failed_subvols,
17b94a
                               priv->child_count);
17b94a
     if (failure_count == priv->child_count) {
17b94a
-        afr_handle_lock_acquire_failure(local, _gf_true);
17b94a
+        afr_handle_lock_acquire_failure(local);
17b94a
         return 0;
17b94a
     } else {
17b94a
         lock = &local->inode_ctx->lock[local->transaction.type];
17b94a
@@ -2092,7 +2089,7 @@ err:
17b94a
     local->op_ret = -1;
17b94a
     local->op_errno = op_errno;
17b94a
 
17b94a
-    afr_handle_lock_acquire_failure(local, _gf_true);
17b94a
+    afr_handle_lock_acquire_failure(local);
17b94a
 
17b94a
     if (xdata_req)
17b94a
         dict_unref(xdata_req);
17b94a
@@ -2361,7 +2358,7 @@ afr_internal_lock_finish(call_frame_t *frame, xlator_t *this)
17b94a
     } else {
17b94a
         lock = &local->inode_ctx->lock[local->transaction.type];
17b94a
         if (local->internal_lock.lock_op_ret < 0) {
17b94a
-            afr_handle_lock_acquire_failure(local, _gf_false);
17b94a
+            afr_handle_lock_acquire_failure(local);
17b94a
         } else {
17b94a
             lock->event_generation = local->event_generation;
17b94a
             afr_changelog_pre_op(frame, this);
17b94a
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
17b94a
index 2cc3797..e731cfa 100644
17b94a
--- a/xlators/cluster/afr/src/afr.h
17b94a
+++ b/xlators/cluster/afr/src/afr.h
17b94a
@@ -1091,8 +1091,8 @@ afr_cleanup_fd_ctx(xlator_t *this, fd_t *fd);
17b94a
 #define AFR_FRAME_INIT(frame, op_errno)                                        \
17b94a
     ({                                                                         \
17b94a
         frame->local = mem_get0(THIS->local_pool);                             \
17b94a
-        if (afr_local_init(frame->local, THIS->private, &op_errno)) {          \
17b94a
-            afr_local_cleanup(frame->local, THIS);                             \
17b94a
+        if (afr_local_init(frame->local, frame->this->private, &op_errno)) {   \
17b94a
+            afr_local_cleanup(frame->local, frame->this);                      \
17b94a
             mem_put(frame->local);                                             \
17b94a
             frame->local = NULL;                                               \
17b94a
         };                                                                     \
17b94a
-- 
17b94a
1.8.3.1
17b94a