From d2c8c165deb86082a13c15cc33d558336ff22d14 Mon Sep 17 00:00:00 2001

From: Ravishankar N <ravishankar@redhat.com>

Date: Tue, 4 Aug 2015 18:37:47 +0530

Subject: [PATCH 294/304] afr: modify afr_txn_nothing_failed()

Patch in master: http://review.gluster.org/#/c/11827/

Patch in release-3.7: http://review.gluster.org/#/c/11985/

In an AFR transaction, we need to consider something as failed only if the

failure (either in the pre-op or the FOP phase) occurs on the bricks on which a

transaction lock was obtained.

Without this, we would end up considering the transaction as failure even on the

bricks on which the lock was not obtained, resulting in unnecessary fsyncs

during the post-op phase of every write transaction for non-appending writes.

Change-Id: I02efa64f6cf80fcc3cb8dd6a9543dbf480b866b9

BUG: 1227759

Signed-off-by: Ravishankar N <ravishankar@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/56256

Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

---

 tests/bugs/replicate/bug-1250170-fsync.c  |   56 +++++++++++++++++++++++++++++

 tests/bugs/replicate/bug-1250170-fsync.t  |   35 ++++++++++++++++++

 xlators/cluster/afr/src/afr-transaction.c |   15 ++------

 3 files changed, 94 insertions(+), 12 deletions(-)

 create mode 100644 tests/bugs/replicate/bug-1250170-fsync.c

 create mode 100644 tests/bugs/replicate/bug-1250170-fsync.t

diff --git a/tests/bugs/replicate/bug-1250170-fsync.c b/tests/bugs/replicate/bug-1250170-fsync.c

new file mode 100644

index 0000000..1d3025b

--- /dev/null

+++ b/tests/bugs/replicate/bug-1250170-fsync.c

@@ -0,0 +1,56 @@

+#include <stdio.h>

+#include <stdlib.h>

+#include <inttypes.h>

+#include <sys/types.h>

+#include <sys/stat.h>

+#include <fcntl.h>

+#include <unistd.h>

+#include <string.h>

+

+int main (int argc, char **argv)

+{

+        char *file = NULL;

+        int fd = -1;

+        char *buffer = NULL;

+        size_t buf_size = 0;

+        size_t written = 0;

+        int ret = 0;

+        off_t offset = 0;

+        int i = 0;

+        int loop_count = 5;

+

+        if (argc < 2) {

+                printf ("Usage:%s <filename>\n", argv[0]);

+                return -1;

+        }

+

+        file = argv[1];

+        buf_size = 1024;

+        buffer = calloc(1, buf_size);

+        if (!buffer) {

+                perror("calloc");

+                return -1;

+        }

+        memset (buffer, 'R', buf_size);

+

+        fd = open(file, O_WRONLY);

+        if (fd == -1) {

+                perror("open");

+                return -1;

+        }

+

+        for (i = 0; i < loop_count; i++) {

+                ret =  write (fd, buffer, buf_size);

+                if (ret == -1) {

+                        perror("write");

+                        return ret;

+                } else {

+                        written += ret;

+                }

+                offset = lseek (fd, 0 , SEEK_SET);

+        }

+

+        free(buffer);

+        return 0;

+

+}

diff --git a/tests/bugs/replicate/bug-1250170-fsync.t b/tests/bugs/replicate/bug-1250170-fsync.t

new file mode 100644

index 0000000..7a3fdbf

--- /dev/null

+++ b/tests/bugs/replicate/bug-1250170-fsync.t

@@ -0,0 +1,35 @@

+#!/bin/bash

+

+. $(dirname $0)/../../include.rc

+. $(dirname $0)/../../volume.rc

+

+cleanup;

+TEST gcc  $(dirname $0)/bug-1250170-fsync.c -o  $(dirname $0)/bug-1250170-fsync

+TEST glusterd

+TEST pidof glusterd

+

+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{0,1}

+TEST $CLI volume set $V0 performance.write-behind off

+TEST $CLI volume start $V0

+TEST $GFS --volfile-id=$V0 --volfile-server=$H0 $M0;

+

+TEST touch $M0/file

+TEST kill_brick $V0 $H0 $B0/${V0}0

+TEST gluster volume profile $V0 start

+#Perform 5 non-sequential writes.

+TEST $(dirname $0)/bug-1250170-fsync $M0/file

+

+#Run profile info initially to filter out the interval statistics in the

+#subsequent runs.

+TEST $CLI volume profile $V0 info

+#We get only cumulative statistics.

+write_count=$($CLI volume profile $V0 info | grep WRITE |awk '{count += $8} END {print count}')

+fsync_count=$($CLI volume profile $V0 info | grep FSYNC |awk '{count += $8} END {print count}')

+

+EXPECT "5" echo $write_count

+TEST [ -z $fsync_count ]

+

+TEST $CLI volume profile $V0 stop

+TEST umount $M0

+rm -f $(dirname $0)/bug-1250170-fsync

+cleanup

diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c

index ba50abd..d773b5a 100644

--- a/xlators/cluster/afr/src/afr-transaction.c

+++ b/xlators/cluster/afr/src/afr-transaction.c

@@ -493,18 +493,14 @@ afr_txn_nothing_failed (call_frame_t *frame, xlator_t *this)

 {

         afr_private_t *priv = NULL;

         afr_local_t *local = NULL;

-        int pre_op_count = 0;

         int i = 0;

         local = frame->local;

 	priv = this->private;

-        pre_op_count = AFR_COUNT (local->transaction.pre_op, priv->child_count);

-        if (pre_op_count < priv->child_count)

-                return _gf_false;

-

         for (i = 0; i < priv->child_count; i++) {

-                if (local->transaction.failed_subvols[i])

+                if (local->transaction.pre_op[i] &&

+                    local->transaction.failed_subvols[i])

                         return _gf_false;

         }

@@ -1156,14 +1152,9 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this)

 		goto err;

 	}

-	if (call_count > 1 &&

-	    (local->transaction.type == AFR_DATA_TRANSACTION ||

+	if ((local->transaction.type == AFR_DATA_TRANSACTION ||

 	     !local->optimistic_change_log)) {

-		/* If we are performing change on only one subvol, no

-		   need to mark dirty, because we are setting the pending

-		   counts already anyways

-		*/

 		local->dirty[idx] = hton32(1);

 		ret = dict_set_static_bin (xdata_req, AFR_DIRTY, local->dirty,

-- 

1.7.1