glusterfs
glusterfs copied to clipboard
gluster
Locks: Optimize the lock flow when fd's are not correctly reopened after a brick reconnection
When the brick is reconnected, we will use the synchronization process to open the reconnected brick before locking.
Fixes:#3183
Signed-off-by: JamesWSWu
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index fadd3e15d..e49988452 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5136,8 +5136,7 @@ afr_lk_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret,
if (child_index < priv->child_count) {
local->child_index = child_index;
- synctask_new(this->ctx->env, afr_lk_txn,
- afr_lk_txn_cbk, frame, frame);
+ synctask_new(this->ctx->env, afr_lk_txn, afr_lk_txn_cbk, frame, frame);
} else if (priv->quorum_count &&
!afr_has_quorum(local->cont.lk.locked_nodes, this, NULL)) {
local->op_ret = -1;
@@ -5155,7 +5154,9 @@ afr_lk_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret,
return 0;
}
-int afr_lk_txn(void *opaque) {
+int
+afr_lk_txn(void *opaque)
+{
call_frame_t *frame = NULL;
afr_local_t *local = NULL;
int child_index = -1;
@@ -5179,7 +5180,6 @@ int afr_lk_txn(void *opaque) {
local->xdata_req);
}
-
int
afr_lk_transaction_cbk(int ret, call_frame_t *frame, void *opaque)
{
diff --git a/xlators/cluster/afr/src/afr-open.c b/xlators/cluster/afr/src/afr-open.c
index 4cbbb3325..8c2cc958b 100644
--- a/xlators/cluster/afr/src/afr-open.c
+++ b/xlators/cluster/afr/src/afr-open.c
@@ -315,10 +315,11 @@ afr_do_fix_open(call_frame_t *frame, xlator_t *this)
gf_msg_debug(this->name, 0, "opening fd for dir %s on subvolume %s",
local->loc.path, priv->children[i]->name);
if (local->is_sync_open) {
- syncop_opendir(priv->children[i], &local->loc, local->fd, NULL, NULL);
+ syncop_opendir(priv->children[i], &local->loc, local->fd, NULL,
+ NULL);
} else {
- STACK_WIND_COOKIE(frame, afr_openfd_fix_open_cbk, (void *)(long)i,
- priv->children[i],
+ STACK_WIND_COOKIE(frame, afr_openfd_fix_open_cbk,
+ (void *)(long)i, priv->children[i],
priv->children[i]->fops->opendir, &local->loc,
local->fd, NULL);
}
@@ -328,15 +329,17 @@ afr_do_fix_open(call_frame_t *frame, xlator_t *this)
local->loc.path, priv->children[i]->name);
if (local->is_sync_open) {
- syncop_open(priv->children[i], &local->loc,
- local->fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC),
- local->fd, NULL, NULL);
+ syncop_open(
+ priv->children[i], &local->loc,
+ local->fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC),
+ local->fd, NULL, NULL);
} else {
STACK_WIND_COOKIE(
frame, afr_openfd_fix_open_cbk, (void *)(long)i,
- priv->children[i], priv->children[i]->fops->open, &local->loc,
- local->fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC), local->fd,
- NULL);
+ priv->children[i], priv->children[i]->fops->open,
+ &local->loc,
+ local->fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC),
+ local->fd, NULL);
}
}
if (!--need_open_count)
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index 861e94e59..0887f3778 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5137,11 +5137,12 @@ afr_lk_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret,
if (child_index < priv->child_count) {
local->child_index = child_index;
- ret = synctask_new(this->ctx->env, afr_lk_txn, afr_lk_txn_cbk, frame, frame);
+ ret = synctask_new(this->ctx->env, afr_lk_txn, afr_lk_txn_cbk, frame,
+ frame);
if (ret) {
- gf_log(this->name, GF_LOG_ERROR,
- "Failed to spawn "
- "new synctask");
+ gf_log(this->name, GF_LOG_ERROR,
+ "Failed to spawn "
+ "new synctask");
}
} else if (priv->quorum_count &&
!afr_has_quorum(local->cont.lk.locked_nodes, this, NULL)) {
@@ -5179,8 +5180,7 @@ afr_lk_txn(void *opaque)
afr_fix_open(fd, this, true);
- STACK_WIND(frame, afr_lk_cbk,
- priv->children[child_index],
+ STACK_WIND(frame, afr_lk_cbk, priv->children[child_index],
priv->children[child_index]->fops->lk, local->fd,
local->cont.lk.cmd, &local->cont.lk.user_flock,
local->xdata_req);
@@ -5362,8 +5362,7 @@ afr_lk(call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t cmd,
local->child_index = 0;
STACK_WIND(frame, afr_lk_cbk, priv->children[i],
- priv->children[i]->fops->lk, fd, cmd, flock,
- local->xdata_req);
+ priv->children[i]->fops->lk, fd, cmd, flock, local->xdata_req);
return 0;
out:
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index eac0198ae..8e8240abd 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5117,7 +5117,8 @@ afr_lk_cbk(call_frame_t *frame, void *cookie, xlator_t *this, int32_t op_ret,
child_index = local->child_index;
- afr_common_lock_cbk(frame, (void *)(long)child_index, this, op_ret, op_errno, xdata);
+ afr_common_lock_cbk(frame, (void *)(long)child_index, this, op_ret,
+ op_errno, xdata);
if (op_ret < 0 && op_errno == EAGAIN) {
local->op_ret = -1;
local->op_errno = EAGAIN;
It is not clear what you are trying to achieve in this PR. Could you provide the exact test that is failing and will pass after this PR? You referenced the test written by Xavi, but how exactly are you running it?
It is not clear what you are trying to achieve in this PR. Could you provide the exact test that is failing and will pass after this PR? You referenced the test written by Xavi, but how exactly are you running it?
@pranithk @xhernandez
Thank you for your review !
- In a two node glusterfs environment, the steps to reproduce the problem are as follows. (1) When the brick processes of the two nodes are connected normally, perform the mount operation. mount -t glusterfs 12.12.12.70,12.12.12.71:4af494b4-8bd8-4a01-ae1e-e54a38aa070e /test (2) Disconnect the brick process of the 12.12.12.70 node. (3) Running program written by Xavi. ./test /test/test-file (4) Wait 5 minutes or more. (5) Reconnect the disconnected brick process of the 12.12.12.70 node. Disconnect the brick process of the 12.12.12.71 node (6) EBADFD error occurs after performing the above steps. As follows, [root@paas-controller:/home/ubuntu]$ ./test /test/test-file [root@paas-controller:/home/ubuntu]$ ./test-open /test/test-file 0: Locking 0: Locked 1: Locking 1: Received signal 18 1: fcntl() failed: (11) Resource temporarily unavailable 0: Unlocking 0: Unlocked 2: Locking 2: Locked 1: Locking 1: fcntl() failed: (77) File descriptor in bad state 2: Unlocking 2: fcntl() failed: (77) File descriptor in bad state 1: Unlocking 1: fcntl() failed: (77) File descriptor in bad state 0: Locking 4: Locking 1: Locking 2: Locking 3: Locking 5: Locking 6: Locking 7: Locking ...
2、This problem occurs because the open operation of the file is not performed or the file is not in the open state when the lock operation is performed. So, this modification is to call the synchronous open operation if the file is not in the open state during the lock operation. This modification solves this problem. That is, the problem cannot be reproduced by performing the above reproduction operation. The results of this revision are as follows. [root@paas-controller:/home/ubuntu]$ ./test /test/test-file 0: Locking 0: Locked 1: Locking 1: Received signal 18 1: fcntl() failed: (11) Resource temporarily unavailable 0: Unlocking 0: Unlocked 2: Locking 2: Locked 1: Locking ...
@pranithk @karthik-us could you take a look ?
The logic seems ok to me, but you know AFR better than me and I wonder if it wouldn't be possible to do the same without synctasks, or they aren't too costly in this case ?
1 test(s) failed ./tests/bugs/protocol/bug-1390914.t
0 test(s) generated core
4 test(s) needed retry ./tests/000-flaky/basic_mount-nfs-auth.t ./tests/000-flaky/bugs_nfs_bug-1116503.t ./tests/00-geo-rep/georep-basic-dr-rsync.t ./tests/bugs/protocol/bug-1390914.t
2 flaky test(s) marked as success even though they failed ./tests/000-flaky/basic_mount-nfs-auth.t ./tests/000-flaky/bugs_nfs_bug-1116503.t https://build.gluster.org/job/gh_centos7-regression/2660/
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index f3ce87117..2badf6c44 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5183,12 +5183,10 @@ afr_lk_txn(void *opaque)
fd = local->fd;
ret = fd_ctx_get(fd, priv->children[child_index], &ctx);
- if (!ret)
- {
+ if (!ret) {
fd_ctx = (afr_fd_ctx_t *)(long)ctx;
}
- if(ret < 0 || (fd_ctx && fd_ctx->is_fd_bad))
- {
+ if (ret < 0 || (fd_ctx && fd_ctx->is_fd_bad)) {
afr_fix_open(fd, this, true);
}
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index 3a2451591..3629b8338 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5321,7 +5321,6 @@ afr_lk(call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t cmd,
}
}
-
STACK_WIND_COOKIE(frame, afr_lk_cbk, (void *)(long)0, priv->children[i],
priv->children[i]->fops->lk, fd, cmd, flock,
local->xdata_req);
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index 39b627d14..886c6a808 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5117,8 +5117,7 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
LOCK(&local->fd->lock);
{
fd = fd_ref(local->fd);
- if (!fd->lk_ctx)
- {
+ if (!fd->lk_ctx) {
ret = 0;
goto out;
}
@@ -5126,8 +5125,7 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
UNLOCK(&local->fd->lock);
fd_ctx = afr_fd_ctx_get(fd, this);
- if (!fd_ctx)
- {
+ if (!fd_ctx) {
ret = 0;
goto out;
}
@@ -5135,8 +5133,8 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
for (index = 0; index < priv->child_count; index++) {
if (fd_ctx->opened_on[index] == AFR_FD_OPENED &&
priv->child_up[index] && !local->need_migrate_lock[index]) {
- ret = syncop_fgetxattr(priv->children[index], fd, &lockinfo, GF_XATTR_LOCKINFO_KEY,
- NULL, NULL);
+ ret = syncop_fgetxattr(priv->children[index], fd, &lockinfo,
+ GF_XATTR_LOCKINFO_KEY, NULL, NULL);
if (ret == 0) {
break;
}
@@ -5163,30 +5161,29 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
if (IA_IFDIR == fd->inode->ia_type) {
syncop_opendir(priv->children[index], &local->loc, fd, NULL,
NULL);
- } else
- {
+ } else {
syncop_open(priv->children[index], &local->loc,
- fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC),
- fd, NULL, NULL);
+ fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC), fd,
+ NULL, NULL);
}
}
- ret = syncop_fsetxattr(priv->children[index], fd, lockinfo, 0, NULL, NULL);
- if(ret < 0)
- {
+ ret = syncop_fsetxattr(priv->children[index], fd, lockinfo, 0, NULL,
+ NULL);
+ if (ret < 0) {
continue;
}
is_migrate_success = true;
}
- if (is_migrate_success){
+ if (is_migrate_success) {
ret = 0;
}
out:
- if(fd)
+ if (fd)
fd_unref(fd);
- if(lockinfo != NULL)
+ if (lockinfo != NULL)
dict_unref(lockinfo);
return ret;
}
@@ -6622,8 +6619,8 @@ afr_local_init(afr_local_t *local, afr_private_t *priv, int32_t *op_errno)
goto out;
}
- local->need_migrate_lock = GF_CALLOC(priv->child_count, sizeof(*local->need_migrate_lock),
- gf_afr_mt_char);
+ local->need_migrate_lock = GF_CALLOC(
+ priv->child_count, sizeof(*local->need_migrate_lock), gf_afr_mt_char);
if (!local->need_migrate_lock) {
if (op_errno)
*op_errno = ENOMEM;
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index 0879853ce..d4697b532 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5104,8 +5104,7 @@ afr_is_need_migrate_lock(afr_local_t *local, unsigned int child_count)
unsigned int index = 0;
gf_boolean_t is_need_migrate = _gf_false;
- for (index = 0; index < child_count; index++)
- {
+ for (index = 0; index < child_count; index++) {
if (local->need_migrate_lock[index]) {
is_need_migrate = _gf_true;
break;
@@ -5115,7 +5114,8 @@ afr_is_need_migrate_lock(afr_local_t *local, unsigned int child_count)
}
static dict_t *
-afr_get_lock_info_from_healthy_brick(afr_fd_ctx_t *fd_ctx, fd_t *fd, afr_private_t *priv, afr_local_t *local)
+afr_get_lock_info_from_healthy_brick(afr_fd_ctx_t *fd_ctx, fd_t *fd,
+ afr_private_t *priv, afr_local_t *local)
{
dict_t *lockinfo = NULL;
void *ptr = NULL;
@@ -5173,7 +5173,7 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
goto out;
}
- lockinfo = afr_get_lock_info_from_healthy_brick(fd_ctx, fd, priv, local);
+ lockinfo = afr_get_lock_info_from_healthy_brick(fd_ctx, fd, priv, local);
if (lockinfo == NULL) {
goto out;
}
@@ -5193,7 +5193,8 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
NULL, NULL);
}
}
- ret = syncop_fsetxattr(priv->children[index], fd, lockinfo, 0, NULL, NULL);
+ ret = syncop_fsetxattr(priv->children[index], fd, lockinfo, 0, NULL,
+ NULL);
if (ret == 0) {
is_migrate_success = _gf_true;
local->need_migrate_lock[index] = 0;
@xhernandez @pranithk
My modified code is to copy the lock from the healthy brick to the brick that needs to be reopened.
Please review it again.
Thank you.
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index d63f04a67..e1c9d0dc1 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5118,7 +5118,6 @@ afr_get_lock_info_from_healthy_brick(afr_fd_ctx_t *fd_ctx, fd_t *fd,
}
if (!dict_get(lockinfo, GF_XATTR_LOCKINFO_KEY)) {
-
dict_unref(lockinfo);
lockinfo = NULL;
goto out;
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index d5f92a308..4b14bf63c 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5177,12 +5177,12 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
if (fd_ctx->opened_on[index] == AFR_FD_NOT_OPENED) {
if (IA_IFDIR == fd->inode->ia_type) {
- ret = syncop_opendir(priv->children[index], &local->loc, fd, NULL,
- NULL);
+ ret = syncop_opendir(priv->children[index], &local->loc, fd,
+ NULL, NULL);
} else {
ret = syncop_open(priv->children[index], &local->loc,
- fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC), fd,
- NULL, NULL);
+ fd_ctx->flags & ~(O_CREAT | O_EXCL | O_TRUNC),
+ fd, NULL, NULL);
}
if (ret) {
CLANG-FORMAT FAILURE: Before merging the patch, this diff needs to be considered for passing clang-format
index cc770402e..7ff62748e 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -5216,7 +5216,7 @@ afr_migrate_locks(afr_private_t *priv, afr_local_t *local)
if (migration_success >= priv->quorum_count) {
ret = 0;
} else {
- fd_close(fd);
+ fd_close(fd);
}
out: