diff options
| author | Ravishankar N <ravishankar@redhat.com> | 2016-12-30 14:57:17 +0530 | 
|---|---|---|
| committer | Pranith Kumar Karampuri <pkarampu@redhat.com> | 2017-01-13 00:31:12 -0800 | 
| commit | deaefaeaf1a2ab136c76cc8e2311233637c9c5d7 (patch) | |
| tree | b8cfd5939d7bd19beb39d1d09df984327693ccdd | |
| parent | 60acdbd87a2c3d5bf1b199b91a7d154467fe1b79 (diff) | |
afr: Avoid resetting event_gen when brick is always down
Problem:
__afr_set_in_flight_sb_status(), which resets event_gen to zero, is
called if failed_subvols[i] is non-zero for any brick. But failed_subvols[i]
is true even if the brick was down *before* the transaction started.
Hence say if 1 brick is down in  a replica-3, every writev that comes
will trigger an inode refresh because of this resetting, as seen from
the no. of FSTATs in the profile info in the BZ.
Fix:
Reset event gen only if the brick was previously a valid read child and
the FOP failed on it the first time.
Also `s/afr_inode_read_subvol_reset/afr_inode_event_gen_reset` because
the function only resets event gen and not the data/metadata readable.
> Signed-off-by: Ravishankar N <ravishankar@redhat.com>
> Reviewed-on: http://review.gluster.org/16309
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
> Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
(cherry picked from commit 522640be476a3f97dac932f7046f0643ec0ec2f2)
Change-Id: I603ae646cbde96995c35db77916e2ed80b602a91
BUG: 1412886
Reviewed-on: http://review.gluster.org/16385
Tested-by: Ravishankar N <ravishankar@redhat.com>
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
| -rw-r--r-- | xlators/cluster/afr/src/afr-common.c | 24 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr-dir-write.c | 6 | ||||
| -rw-r--r-- | xlators/cluster/afr/src/afr.h | 6 | 
3 files changed, 17 insertions, 19 deletions
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c index 739f6abfb1e..86892ea6f06 100644 --- a/xlators/cluster/afr/src/afr-common.c +++ b/xlators/cluster/afr/src/afr-common.c @@ -155,6 +155,7 @@ __afr_inode_ctx_get (xlator_t *this, inode_t *inode, afr_inode_ctx_t **ctx)  out:          return ret;  } +  /*   * INODE CTX 64-bit VALUE FORMAT FOR SMALL (<= 16) SUBVOL COUNTS:   * @@ -216,10 +217,7 @@ __afr_set_in_flight_sb_status (xlator_t *this, afr_local_t *local,          metadatamap_old = metadatamap = (val & 0x000000000000ffff);          datamap_old = datamap = (val & 0x00000000ffff0000) >> 16; -        /* Hard-code event to 0 since there is a failure and the inode -         * needs to be refreshed anyway. -         */ -        event = 0; +        event = (val & 0xffffffff00000000) >> 32;          if (txn_type == AFR_DATA_TRANSACTION)                  tmp_map = datamap; @@ -252,6 +250,8 @@ __afr_set_in_flight_sb_status (xlator_t *this, afr_local_t *local,                          local->transaction.in_flight_sb = _gf_true;                          metadatamap |= (1 << index);                  } +                if (metadatamap_old != metadatamap) +                        event = 0;                  break;          case AFR_DATA_TRANSACTION: @@ -261,10 +261,12 @@ __afr_set_in_flight_sb_status (xlator_t *this, afr_local_t *local,                          local->transaction.in_flight_sb = _gf_true;                          datamap |= (1 << index);                  } +                if (datamap_old != datamap) +                        event = 0;                  break;          default: -        break; +                break;          }          val = ((uint64_t) metadatamap) | @@ -375,7 +377,7 @@ out:  }  int -__afr_inode_read_subvol_reset_small (inode_t *inode, xlator_t *this) +__afr_inode_event_gen_reset_small (inode_t *inode, xlator_t *this)  {  	int               ret         = -1;  	uint16_t          datamap     = 0; @@ -476,7 +478,7 @@ out:  }  int -__afr_inode_read_subvol_reset (inode_t *inode, xlator_t *this) +__afr_inode_event_gen_reset (inode_t *inode, xlator_t *this)  {  	afr_private_t *priv = NULL;  	int ret = -1; @@ -484,7 +486,7 @@ __afr_inode_read_subvol_reset (inode_t *inode, xlator_t *this)  	priv = this->private;  	if (priv->child_count <= 16) -		ret = __afr_inode_read_subvol_reset_small (inode, this); +		ret = __afr_inode_event_gen_reset_small (inode, this);  	else  		ret = -1; @@ -673,7 +675,7 @@ out:  }  int -afr_inode_read_subvol_reset (inode_t *inode, xlator_t *this) +afr_inode_event_gen_reset (inode_t *inode, xlator_t *this)  {  	int ret = -1; @@ -681,7 +683,7 @@ afr_inode_read_subvol_reset (inode_t *inode, xlator_t *this)          LOCK(&inode->lock);          { -                ret = __afr_inode_read_subvol_reset (inode, this); +                ret = __afr_inode_event_gen_reset (inode, this);          }          UNLOCK(&inode->lock);  out: @@ -2209,7 +2211,7 @@ afr_lookup_done (call_frame_t *frame, xlator_t *this)  		if (afr_replies_interpret (frame, this, local->inode, NULL)) {                          read_subvol = afr_read_subvol_decide (local->inode,                                                                this, &args); -			afr_inode_read_subvol_reset (local->inode, this); +			afr_inode_event_gen_reset (local->inode, this);  			goto cant_interpret;  		} else {                          read_subvol = afr_data_subvol_get (local->inode, this, diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c index 286a5392da6..8e483c382c4 100644 --- a/xlators/cluster/afr/src/afr-dir-write.c +++ b/xlators/cluster/afr/src/afr-dir-write.c @@ -122,12 +122,12 @@ __afr_dir_write_finalize (call_frame_t *frame, xlator_t *this)  			continue;  		if (local->replies[i].op_ret < 0) {  			if (local->inode) -				afr_inode_read_subvol_reset (local->inode, this); +				afr_inode_event_gen_reset (local->inode, this);  			if (local->parent) -				afr_inode_read_subvol_reset (local->parent, +				afr_inode_event_gen_reset (local->parent,  							     this);  			if (local->parent2) -				afr_inode_read_subvol_reset (local->parent2, +				afr_inode_event_gen_reset (local->parent2,  							     this);  			continue;  		} diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h index 642d633f9c4..e1df803c172 100644 --- a/xlators/cluster/afr/src/afr.h +++ b/xlators/cluster/afr/src/afr.h @@ -882,7 +882,7 @@ afr_inode_read_subvol_set (inode_t *inode, xlator_t *this,  			   int event_generation);  int -afr_inode_read_subvol_reset (inode_t *inode, xlator_t *this); +afr_inode_event_gen_reset (inode_t *inode, xlator_t *this);  int  afr_read_subvol_select_by_policy (inode_t *inode, xlator_t *this, @@ -906,10 +906,6 @@ afr_read_subvol_get (inode_t *inode, xlator_t *this, int *subvol_p,  	afr_read_subvol_get(i, t, s, r, e, AFR_METADATA_TRANSACTION, a)  int -afr_inode_ctx_reset_unreadable_subvol (inode_t *inode, xlator_t *this, -                                       int subvol_idx, int txn_type); - -int  afr_inode_refresh (call_frame_t *frame, xlator_t *this, inode_t *inode,                     uuid_t gfid, afr_inode_refresh_cbk_t cbk);  | 
