diff options
author | N Balachandran <nbalacha@redhat.com> | 2017-08-04 14:46:38 +0530 |
---|---|---|
committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2017-09-17 12:47:26 +0000 |
commit | de744c3cbb4ec91e4a2f66575d778633700933c1 (patch) | |
tree | 007f1613b51427f7c7d65b712468c7829422a1b5 /xlators/cluster/dht/src/dht-inode-write.c | |
parent | b4eca2f985d8207183f15fccfc2f907737625097 (diff) |
cluster/dht: Check for open fd only on EBADF
DHT fd based fops used to check if the fd was open
on the cached subvol before winding the call. However,
this introduced a performance regression of about
30% for reads.
This check was introduced to handle cases where files
were migrated while IOs were happening. As this is not
the common case, dht will now check if the fd is
open on the cached subvol only if the call fails
with EBADF.
This will prevent a performance hit where a rebalance
is not running.
> BUG: 1476665
> Signed-off-by: N Balachandran <nbalacha@redhat.com>
> Reviewed-on: https://review.gluster.org/17976
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Amar Tumballi <amarts@redhat.com>
> Reviewed-by: Susant Palai <spalai@redhat.com>
> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Change-Id: I2035a858d63c3fcd22bb634055bbb0ad01686808
BUG: 1467010
Signed-off-by: N Balachandran <nbalacha@redhat.com>
Reviewed-on: https://review.gluster.org/18057
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Diffstat (limited to 'xlators/cluster/dht/src/dht-inode-write.c')
-rw-r--r-- | xlators/cluster/dht/src/dht-inode-write.c | 210 |
1 files changed, 110 insertions, 100 deletions
diff --git a/xlators/cluster/dht/src/dht-inode-write.c b/xlators/cluster/dht/src/dht-inode-write.c index f925d4b59ed..5392ee4635b 100644 --- a/xlators/cluster/dht/src/dht-inode-write.c +++ b/xlators/cluster/dht/src/dht-inode-write.c @@ -44,6 +44,19 @@ dht_writev_cbk (call_frame_t *frame, void *cookie, xlator_t *this, goto out; } + /* writev fails with EBADF if dht has not yet opened the fd + * on the cached subvol. This could happen if the file was migrated + * and a lookup updated the cached subvol in the inode ctx. + * We only check once as this could be a valid bad fd error. + */ + + if (op_ret == -1 && (op_errno == EBADF) && !(local->fd_checked)) { + ret = dht_check_and_open_fd_on_subvol (this, frame); + if (ret) + goto out; + return 0; + } + if (op_ret == -1 && !dht_inode_missing(op_errno)) { local->op_errno = op_errno; local->op_ret = -1; @@ -162,7 +175,6 @@ dht_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, xlator_t *subvol = NULL; int op_errno = -1; dht_local_t *local = NULL; - int ret = -1; VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); @@ -183,7 +195,6 @@ dht_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, goto err; } - if (xdata) local->xattr_req = dict_ref (xdata); @@ -194,22 +205,13 @@ dht_writev (call_frame_t *frame, xlator_t *this, fd_t *fd, local->rebalance.iobref = iobref_ref (iobref); local->call_cnt = 1; - if (dht_fd_open_on_dst (this, fd, subvol)) { - - STACK_WIND_COOKIE (frame, dht_writev_cbk, subvol, subvol, - subvol->fops->writev, fd, - local->rebalance.vector, - local->rebalance.count, - local->rebalance.offset, - local->rebalance.flags, - local->rebalance.iobref, local->xattr_req); - return 0; - - } else { - ret = dht_check_and_open_fd_on_subvol (this, frame); - if (ret) - goto err; - } + STACK_WIND_COOKIE (frame, dht_writev_cbk, subvol, subvol, + subvol->fops->writev, fd, + local->rebalance.vector, + local->rebalance.count, + local->rebalance.offset, + local->rebalance.flags, + local->rebalance.iobref, local->xattr_req); return 0; @@ -243,6 +245,22 @@ dht_truncate_cbk (call_frame_t *frame, void *cookie, xlator_t *this, local = frame->local; prev = cookie; + /* Needs to be checked only for ftruncate. + * ftruncate fails with EBADF/EINVAL if dht has not yet opened the fd + * on the cached subvol. This could happen if the file was migrated + * and a lookup updated the cached subvol in the inode ctx. + * We only check once as this could actually be a valid error. + */ + + if ((local->fop == GF_FOP_FTRUNCATE) && (op_ret == -1) + && ((op_errno == EBADF) || (op_errno == EINVAL)) + && !(local->fd_checked)) { + ret = dht_check_and_open_fd_on_subvol (this, frame); + if (ret) + goto out; + return 0; + } + if ((op_ret == -1) && !dht_inode_missing(op_errno)) { local->op_errno = op_errno; local->op_ret = -1; @@ -253,6 +271,7 @@ dht_truncate_cbk (call_frame_t *frame, void *cookie, xlator_t *this, goto out; } + if (local->call_cnt != 1) { if (local->stbuf.ia_blocks) { dht_iatt_merge (this, postbuf, &local->stbuf, NULL); @@ -408,7 +427,6 @@ dht_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, xlator_t *subvol = NULL; int op_errno = -1; dht_local_t *local = NULL; - int ret = -1; VALIDATE_OR_GOTO (frame, err); @@ -434,20 +452,9 @@ dht_ftruncate (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, if (xdata) local->xattr_req = dict_ref (xdata); - if (dht_fd_open_on_dst (this, fd, subvol)) { - - STACK_WIND_COOKIE (frame, dht_truncate_cbk, subvol, subvol, - subvol->fops->ftruncate, fd, - local->rebalance.offset, local->xattr_req); - return 0; - - } else { - - ret = dht_check_and_open_fd_on_subvol (this, frame); - if (ret) - goto err; - } - + STACK_WIND_COOKIE (frame, dht_truncate_cbk, subvol, subvol, + subvol->fops->ftruncate, fd, + local->rebalance.offset, local->xattr_req); return 0; err: @@ -477,6 +484,20 @@ dht_fallocate_cbk(call_frame_t *frame, void *cookie, xlator_t *this, local = frame->local; prev = cookie; + /* fallocate fails with EBADF if dht has not yet opened the fd + * on the cached subvol. This could happen if the file was migrated + * and a lookup updated the cached subvol in the inode ctx. + * We only check once as this could actually be a valid error. + */ + + if ((op_ret == -1) && (op_errno == EBADF) + && !(local->fd_checked)) { + ret = dht_check_and_open_fd_on_subvol (this, frame); + if (ret) + goto out; + return 0; + } + if ((op_ret == -1) && !dht_inode_missing(op_errno)) { local->op_errno = op_errno; local->op_ret = -1; @@ -586,7 +607,6 @@ dht_fallocate (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t mode, xlator_t *subvol = NULL; int op_errno = -1; dht_local_t *local = NULL; - int ret = -1; VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); @@ -614,22 +634,12 @@ dht_fallocate (call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t mode, if (xdata) local->xattr_req = dict_ref (xdata); - if (dht_fd_open_on_dst (this, fd, subvol)) { - - STACK_WIND_COOKIE (frame, dht_fallocate_cbk, subvol, subvol, - subvol->fops->fallocate, fd, - local->rebalance.flags, - local->rebalance.offset, - local->rebalance.size, - local->xattr_req); - return 0; - - } else { - - ret = dht_check_and_open_fd_on_subvol (this, frame); - if (ret) - goto err; - } + STACK_WIND_COOKIE (frame, dht_fallocate_cbk, subvol, subvol, + subvol->fops->fallocate, fd, + local->rebalance.flags, + local->rebalance.offset, + local->rebalance.size, + local->xattr_req); return 0; @@ -660,6 +670,20 @@ dht_discard_cbk(call_frame_t *frame, void *cookie, xlator_t *this, local = frame->local; prev = cookie; + + /* discard fails with EBADF if dht has not yet opened the fd + * on the cached subvol. This could happen if the file was migrated + * and a lookup updated the cached subvol in the inode ctx. + * We only check once as this could actually be a valid error. + */ + if ((op_ret == -1) && (op_errno == EBADF) + && !(local->fd_checked)) { + ret = dht_check_and_open_fd_on_subvol (this, frame); + if (ret) + goto out; + return 0; + } + if ((op_ret == -1) && !dht_inode_missing(op_errno)) { local->op_errno = op_errno; local->op_ret = -1; @@ -769,7 +793,6 @@ dht_discard (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, xlator_t *subvol = NULL; int op_errno = -1; dht_local_t *local = NULL; - int ret = -1; VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); @@ -796,22 +819,11 @@ dht_discard (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, if (xdata) local->xattr_req = dict_ref (xdata); - if (dht_fd_open_on_dst (this, fd, subvol)) { - - STACK_WIND_COOKIE (frame, dht_discard_cbk, subvol, subvol, - subvol->fops->discard, fd, - local->rebalance.offset, - local->rebalance.size, - local->xattr_req); - return 0; - - } else { - - ret = dht_check_and_open_fd_on_subvol (this, frame); - if (ret) - goto err; - - } + STACK_WIND_COOKIE (frame, dht_discard_cbk, subvol, subvol, + subvol->fops->discard, fd, + local->rebalance.offset, + local->rebalance.size, + local->xattr_req); return 0; @@ -840,6 +852,19 @@ dht_zerofill_cbk(call_frame_t *frame, void *cookie, xlator_t *this, local = frame->local; prev = cookie; + /* zerofill fails with EBADF if dht has not yet opened the fd + * on the cached subvol. This could happen if the file was migrated + * and a lookup updated the cached subvol in the inode ctx. + * We only check once as this could actually be a valid error. + */ + if ((op_ret == -1) && (op_errno == EBADF) + && !(local->fd_checked)) { + ret = dht_check_and_open_fd_on_subvol (this, frame); + if (ret) + goto out; + return 0; + } + if ((op_ret == -1) && !dht_inode_missing(op_errno)) { local->op_errno = op_errno; local->op_ret = -1; @@ -952,7 +977,6 @@ dht_zerofill (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, xlator_t *subvol = NULL; int op_errno = -1; dht_local_t *local = NULL; - int ret = -1; VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); @@ -979,21 +1003,10 @@ dht_zerofill (call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset, if (xdata) local->xattr_req = dict_ref (xdata); - if (dht_fd_open_on_dst (this, fd, subvol)) { - - STACK_WIND_COOKIE (frame, dht_zerofill_cbk, subvol, subvol, - subvol->fops->zerofill, fd, - local->rebalance.offset, - local->rebalance.size, local->xattr_req); - return 0; - - } else { - - ret = dht_check_and_open_fd_on_subvol (this, frame); - if (ret) - goto err; - } - + STACK_WIND_COOKIE (frame, dht_zerofill_cbk, subvol, subvol, + subvol->fops->zerofill, fd, + local->rebalance.offset, + local->rebalance.size, local->xattr_req); return 0; @@ -1020,6 +1033,15 @@ dht_file_setattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this, prev = cookie; local->op_errno = op_errno; + + if ((op_ret == -1) && (op_errno == EBADF) + && !(local->fd_checked)) { + ret = dht_check_and_open_fd_on_subvol (this, frame); + if (ret) + goto out; + return 0; + } + if ((op_ret == -1) && !dht_inode_missing(op_errno)) { gf_msg_debug (this->name, op_errno, "subvolume %s returned -1", @@ -1241,8 +1263,6 @@ dht_fsetattr (call_frame_t *frame, xlator_t *this, fd_t *fd, struct iatt *stbuf, int op_errno = -1; int i = -1; int call_cnt = 0; - int ret = -1; - VALIDATE_OR_GOTO (frame, err); VALIDATE_OR_GOTO (this, err); @@ -1279,21 +1299,11 @@ dht_fsetattr (call_frame_t *frame, xlator_t *this, fd_t *fd, struct iatt *stbuf, local->call_cnt = 1; subvol = local->cached_subvol; - if (dht_fd_open_on_dst (this, fd, subvol)) { - - STACK_WIND_COOKIE (frame, dht_file_setattr_cbk, subvol, - subvol, subvol->fops->fsetattr, fd, - &local->rebalance.stbuf, - local->rebalance.flags, - local->xattr_req); - return 0; - - } else { - ret = dht_check_and_open_fd_on_subvol (this, frame); - if (ret) - goto err; - - } + STACK_WIND_COOKIE (frame, dht_file_setattr_cbk, subvol, + subvol, subvol->fops->fsetattr, fd, + &local->rebalance.stbuf, + local->rebalance.flags, + local->xattr_req); return 0; } |