summaryrefslogtreecommitdiffstats
path: root/xlators/cluster/afr
diff options
context:
space:
mode:
authorPranith Kumar K <pkarampu@redhat.com>2016-03-19 11:40:26 +0530
committerPranith Kumar Karampuri <pkarampu@redhat.com>2016-05-04 18:42:14 -0700
commit4c4624c9bad2edf27128cb122c64f15d7d63bbc8 (patch)
tree71e77a46f2a793c3fe1fcd4256f25ccd26b369e5 /xlators/cluster/afr
parente3b2ed938a5dc8a72d1fe3d7ced341646d241ca4 (diff)
cluster/afr: Don't let NFS cache stat after writes
Problem: Afr does post-ops after write but the stat buffer it unwinds is at the time of write, so if nfs client caches this, it will see different ctime when it does stat on it after post-op is done. From NFS client's perspective it thinks the file is changed. Tar which depends on this to be correct keeps giving 'file changed as we read it' warning. If Afr instead has to choose to unwind after post-op, eager-lock, delayed-post-op will have to be disabled which will lead to bad performance for all write usecases. Fix: Don't let client cache stat after write. Change-Id: Ic6062acc6e5cdd97a9c83c56bd529ec83cee8a23 BUG: 1302948 Signed-off-by: Pranith Kumar K <pkarampu@redhat.com> Signed-off-by: Anuradha Talur <atalur@redhat.com> Reviewed-on: http://review.gluster.org/13785 Smoke: Gluster Build System <jenkins@build.gluster.com> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> CentOS-regression: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Niels de Vos <ndevos@redhat.com>
Diffstat (limited to 'xlators/cluster/afr')
-rw-r--r--xlators/cluster/afr/src/afr-dir-write.c11
-rw-r--r--xlators/cluster/afr/src/afr-inode-write.c16
-rw-r--r--xlators/cluster/afr/src/afr-transaction.c46
-rw-r--r--xlators/cluster/afr/src/afr-transaction.h2
4 files changed, 69 insertions, 6 deletions
diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c
index 62ab54ae0ca..55aec7429a7 100644
--- a/xlators/cluster/afr/src/afr-dir-write.c
+++ b/xlators/cluster/afr/src/afr-dir-write.c
@@ -234,7 +234,9 @@ __afr_dir_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
afr_local_t *local = NULL;
int child_index = (long) cookie;
int call_count = -1;
+ afr_private_t *priv = NULL;
+ priv = this->private;
local = frame->local;
LOCK (&frame->lock);
@@ -249,8 +251,13 @@ __afr_dir_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
if (call_count == 0) {
__afr_dir_write_finalize (frame, this);
- if (afr_txn_nothing_failed (frame, this))
- local->transaction.unwind (frame, this);
+ if (afr_txn_nothing_failed (frame, this)) {
+ /*if it did pre-op, it will do post-op changing ctime*/
+ if (priv->consistent_metadata &&
+ afr_needs_changelog_update (local))
+ afr_zero_fill_stat (local);
+ local->transaction.unwind (frame, this);
+ }
afr_mark_entry_pending_changelog (frame, this);
diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c
index f240b5eec39..36889429657 100644
--- a/xlators/cluster/afr/src/afr-inode-write.c
+++ b/xlators/cluster/afr/src/afr-inode-write.c
@@ -182,7 +182,9 @@ __afr_inode_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
afr_local_t *local = NULL;
int child_index = (long) cookie;
int call_count = -1;
+ afr_private_t *priv = NULL;
+ priv = this->private;
local = frame->local;
LOCK (&frame->lock);
@@ -198,8 +200,13 @@ __afr_inode_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
if (call_count == 0) {
__afr_inode_write_finalize (frame, this);
- if (afr_txn_nothing_failed (frame, this))
- local->transaction.unwind (frame, this);
+ if (afr_txn_nothing_failed (frame, this)) {
+ /*if it did pre-op, it will do post-op changing ctime*/
+ if (priv->consistent_metadata &&
+ afr_needs_changelog_update (local))
+ afr_zero_fill_stat (local);
+ local->transaction.unwind (frame, this);
+ }
local->transaction.resume (frame, this);
}
@@ -230,8 +237,13 @@ void
afr_writev_unwind (call_frame_t *frame, xlator_t *this)
{
afr_local_t * local = NULL;
+ afr_private_t *priv = this->private;
+
local = frame->local;
+ if (priv->consistent_metadata)
+ afr_zero_fill_stat (local);
+
AFR_STACK_UNWIND (writev, frame,
local->op_ret, local->op_errno,
&local->cont.inode_wfop.prebuf,
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index 2acac027122..6fd44ce79f6 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -36,6 +36,37 @@ afr_changelog_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,
afr_changelog_resume_t changelog_resume,
afr_xattrop_type_t op);
+void
+afr_zero_fill_stat (afr_local_t *local)
+{
+ if (!local)
+ return;
+ if (local->transaction.type == AFR_DATA_TRANSACTION ||
+ local->transaction.type == AFR_METADATA_TRANSACTION) {
+ gf_zero_fill_stat (&local->cont.inode_wfop.prebuf);
+ gf_zero_fill_stat (&local->cont.inode_wfop.postbuf);
+ } else if (local->transaction.type == AFR_ENTRY_TRANSACTION ||
+ local->transaction.type == AFR_ENTRY_RENAME_TRANSACTION) {
+ gf_zero_fill_stat (&local->cont.dir_fop.buf);
+ gf_zero_fill_stat (&local->cont.dir_fop.preparent);
+ gf_zero_fill_stat (&local->cont.dir_fop.postparent);
+ if (local->transaction.type == AFR_ENTRY_TRANSACTION)
+ return;
+ gf_zero_fill_stat (&local->cont.dir_fop.prenewparent);
+ gf_zero_fill_stat (&local->cont.dir_fop.postnewparent);
+ }
+}
+
+gf_boolean_t
+afr_needs_changelog_update (afr_local_t *local)
+{
+ if (local->transaction.type == AFR_DATA_TRANSACTION)
+ return _gf_true;
+ if (!local->optimistic_change_log)
+ return _gf_true;
+ return _gf_false;
+}
+
static int32_t
afr_quorum_errno (afr_private_t *priv)
{
@@ -85,9 +116,21 @@ int
__afr_txn_write_done (call_frame_t *frame, xlator_t *this)
{
afr_local_t *local = NULL;
+ afr_private_t *priv = NULL;
+ gf_boolean_t unwind = _gf_false;
+ priv = this->private;
local = frame->local;
+ if (priv->consistent_metadata) {
+ LOCK (&frame->lock);
+ {
+ unwind = (local->transaction.main_frame != NULL);
+ }
+ UNLOCK (&frame->lock);
+ if (unwind)/*It definitely did post-op*/
+ afr_zero_fill_stat (local);
+ }
local->transaction.unwind (frame, this);
AFR_STACK_DESTROY (frame);
@@ -1232,8 +1275,7 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this)
goto err;
}
- if ((local->transaction.type == AFR_DATA_TRANSACTION ||
- !local->optimistic_change_log)) {
+ if (afr_needs_changelog_update (local)) {
local->dirty[idx] = hton32(1);
diff --git a/xlators/cluster/afr/src/afr-transaction.h b/xlators/cluster/afr/src/afr-transaction.h
index 47d43d88991..c58531eff44 100644
--- a/xlators/cluster/afr/src/afr-transaction.h
+++ b/xlators/cluster/afr/src/afr-transaction.h
@@ -52,5 +52,7 @@ int __afr_txn_write_fop (call_frame_t *frame, xlator_t *this);
int __afr_txn_write_done (call_frame_t *frame, xlator_t *this);
call_frame_t *afr_transaction_detach_fop_frame (call_frame_t *frame);
gf_boolean_t afr_has_quorum (unsigned char *subvols, xlator_t *this);
+gf_boolean_t afr_needs_changelog_update (afr_local_t *local);
+void afr_zero_fill_stat (afr_local_t *local);
#endif /* __TRANSACTION_H__ */