From 1ceab6768cc5dfc50b0c00e07655d907cf489f09 Mon Sep 17 00:00:00 2001

From: Pranith Kumar K <pkarampu@redhat.com>

Date: Wed, 24 May 2017 22:30:29 +0530

Subject: [PATCH 480/486] features/shard: Handle offset in appending writes

When a file is opened with append, all writes are appended at the end of file

irrespective of the offset given in the write syscall. This needs to be

considered in shard size update function and also for choosing which shard to

write to.

At the moment shard piggybacks on queuing from write-behind

xlator for ordering of the operations. So if write-behind is disabled and

two parallel appending-writes come both of which can increase the file size

beyond shard-size the file will be corrupted.

 >BUG: 1455301

 >Change-Id: I9007e6a39098ab0b5d5386367bd07eb5f89cb09e

 >Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>

 >Reviewed-on: https://review.gluster.org/17387

 >Smoke: Gluster Build System <jenkins@build.gluster.org>

 >Reviewed-by: Krutika Dhananjay <kdhananj@redhat.com>

 >NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>

 >CentOS-regression: Gluster Build System <jenkins@build.gluster.org>

BUG: 1454313

Change-Id: I9007e6a39098ab0b5d5386367bd07eb5f89cb09e

Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>

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

---

 extras/group-gluster-block           |   1 -

 tests/bugs/shard/shard-append-test.c | 179 +++++++++++++++++++++++++++++++++++

 tests/bugs/shard/shard-append-test.t |  32 +++++++

 xlators/features/shard/src/shard.c   | 109 +++++++++++++--------

 4 files changed, 278 insertions(+), 43 deletions(-)

 create mode 100644 tests/bugs/shard/shard-append-test.c

 create mode 100644 tests/bugs/shard/shard-append-test.t

diff --git a/extras/group-gluster-block b/extras/group-gluster-block

index 0753d26..a4a6367 100644

--- a/extras/group-gluster-block

+++ b/extras/group-gluster-block

@@ -2,7 +2,6 @@ performance.quick-read=off

 performance.read-ahead=off

 performance.io-cache=off

 performance.stat-prefetch=off

-performance.write-behind=off

 performance.open-behind=off

 performance.readdir-ahead=off

 network.remote-dio=enable

diff --git a/tests/bugs/shard/shard-append-test.c b/tests/bugs/shard/shard-append-test.c

new file mode 100644

index 0000000..92dff3d

--- /dev/null

+++ b/tests/bugs/shard/shard-append-test.c

@@ -0,0 +1,179 @@

+#include <fcntl.h>

+#include <unistd.h>

+#include <time.h>

+#include <limits.h>

+#include <string.h>

+#include <pthread.h>

+#include <stdio.h>

+#include <stdlib.h>

+#include <errno.h>

+#include <glusterfs/api/glfs.h>

+#include <glusterfs/api/glfs-handles.h>

+

+#define LOG_ERR(msg) do { \

+        fprintf (stderr, "%s : Error (%s)\n", msg, strerror (errno)); \

+        } while (0)

+

+/*This test tests that shard xlator handles offset in appending writes

+ * correctly. This test performs writes of 1025 bytes 1025 times, in 5 threads

+ * with different threads. The buffer to be written is same character repeated

+ * 1025 times in the buffer for a thread. At the end it reads the buffer till

+ * end of file and tests that the read of 1025 bytes is always same character

+ * and the content read is 5*1025*1025 size. 1025 bytes is chosen because it

+ * will lead to write on more than one shard at some point when the size is

+ * going over the initial shard*/

+pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;

+int thread_data = '1';

+

+glfs_t *

+init_glfs (const char *hostname, const char *volname,

+           const char *logfile)

+{

+        int     ret     = -1;

+        glfs_t *fs      = NULL;

+

+        fs = glfs_new (volname);

+        if (!fs) {

+                LOG_ERR ("glfs_new failed");

+                return NULL;

+        }

+

+        ret = glfs_set_volfile_server (fs, "tcp", hostname, 24007);

+        if (ret < 0) {

+                LOG_ERR ("glfs_set_volfile_server failed");

+                goto out;

+        }

+

+        ret = glfs_set_logging (fs, logfile, 7);

+        if (ret < 0) {

+                LOG_ERR ("glfs_set_logging failed");

+                goto out;

+        }

+

+        ret = glfs_init (fs);

+        if (ret < 0) {

+                LOG_ERR ("glfs_init failed");

+                goto out;

+        }

+

+        ret = 0;

+out:

+        if (ret) {

+                glfs_fini (fs);

+                fs = NULL;

+        }

+

+        return fs;

+}

+

+void*

+write_data (void *data)

+{

+        char           buf[1025] = {0};

+        glfs_fd_t      *glfd = NULL;

+        glfs_t         *fs       = data;

+        int            i     = 0;

+

+        pthread_mutex_lock (&lock);

+        {

+                memset(buf, thread_data, sizeof(buf));

+                thread_data++;

+        }

+        pthread_mutex_unlock (&lock);

+

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

+                glfd = glfs_creat(fs, "parallel-write.txt", O_WRONLY | O_APPEND,

+                                   S_IRUSR | S_IWUSR | O_SYNC);

+                if (!glfd) {

+                        LOG_ERR ("Failed to create file");

+                        exit(1);

+                }

+

+                if (glfs_write (glfd, buf, sizeof(buf), 0) < 0) {

+                        LOG_ERR ("Failed to write to file");

+                        exit(1);

+                }

+                if (glfs_close(glfd) != 0) {

+                        LOG_ERR ("Failed to close file");

+                        exit(1);

+                }

+        }

+        return NULL;

+}

+

+int

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

+{

+        pthread_t  tid[5] = {0};

+        char       buf[1025] = {0};

+        char       cmp_buf[1025] = {0};

+        int         ret      = 0;

+        char       *hostname = NULL;

+        char       *volname  = NULL;

+        char       *logfile  = NULL;

+        glfs_t     *fs       = NULL;

+        glfs_fd_t  *glfd     = NULL;

+        ssize_t     bytes_read = 0;

+        ssize_t     total_bytes_read = 0;

+        int i = 0;

+

+        if (argc != 4) {

+                fprintf (stderr, "Invalid argument\n");

+                exit(1);

+        }

+

+        hostname = argv[1];

+        volname = argv[2];

+        logfile = argv[3];

+

+        fs = init_glfs (hostname, volname, logfile);

+        if (fs == NULL) {

+                LOG_ERR ("init_glfs failed");

+                return -1;

+        }

+

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

+                pthread_create(&tid[i], NULL, write_data, fs);

+        }

+

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

+                pthread_join(tid[i], NULL);

+        }

+        glfd = glfs_open(fs, "parallel-write.txt", O_RDONLY);

+        if (!glfd) {

+                LOG_ERR ("Failed to open file for reading");

+                exit(1);

+        }

+

+        while ((bytes_read = glfs_read (glfd, buf, sizeof(buf), 0)) > 0) {

+                if (bytes_read != sizeof(buf)) {

+                        fprintf (stderr, "Didn't read complete data read: %zd "

+                                 "expected: %lu", bytes_read, sizeof(buf));

+                        exit(1);

+                }

+

+                total_bytes_read += bytes_read;

+                if (buf[0] < '1' || buf[0] >= thread_data) {

+                        fprintf(stderr, "Invalid character found: %c", buf[0]);

+                        exit(1);

+                }

+                memset(cmp_buf, buf[0], sizeof(cmp_buf));

+                if (memcmp(cmp_buf, buf, sizeof(cmp_buf))) {

+                        LOG_ERR ("Data corrupted");

+                        exit(1);

+                }

+                memset(cmp_buf, 0, sizeof(cmp_buf));

+        }

+

+        if (total_bytes_read != 5*1025*1025) {

+                fprintf(stderr, "Failed to read what is written, read; %zd, "

+                        "expected %zu", total_bytes_read, 5*1025*1025);

+                exit(1);

+        }

+

+        if (glfs_close(glfd) != 0) {

+                LOG_ERR ("Failed to close");

+                exit(1);

+        }

+        return 0;

+}

diff --git a/tests/bugs/shard/shard-append-test.t b/tests/bugs/shard/shard-append-test.t

new file mode 100644

index 0000000..f8719f2

--- /dev/null

+++ b/tests/bugs/shard/shard-append-test.t

@@ -0,0 +1,32 @@

+#!/bin/bash

+

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

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

+

+cleanup;

+

+TEST glusterd

+

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

+TEST $CLI volume set $V0 features.shard on

+TEST $CLI volume set $V0 features.shard-block-size 4MB

+TEST $CLI volume set $V0 performance.quick-read off

+TEST $CLI volume set $V0 performance.io-cache off

+

+#Uncomment the following line after shard-queuing is implemented

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

+

+TEST $CLI volume set $V0 performance.strict-o-direct on

+TEST $CLI volume set $V0 performance.stat-prefetch off

+TEST $CLI volume set $V0 performance.read-ahead off

+TEST $CLI volume start $V0;

+

+logdir=`gluster --print-logdir`

+

+TEST build_tester $(dirname $0)/shard-append-test.c -lgfapi -lpthread

+

+TEST ./$(dirname $0)/shard-append-test ${H0} $V0 $logdir/shard-append-test.log

+

+cleanup_tester $(dirname $0)/shard-append-test

+

+cleanup;

diff --git a/xlators/features/shard/src/shard.c b/xlators/features/shard/src/shard.c

index bbd566a..141c42f 100644

--- a/xlators/features/shard/src/shard.c

+++ b/xlators/features/shard/src/shard.c

@@ -3634,6 +3634,18 @@ shard_common_inode_write_post_update_size_handler (call_frame_t *frame,

         return 0;

 }

+static gf_boolean_t

+shard_is_appending_write (shard_local_t *local)

+{

+        if (local->fop != GF_FOP_WRITE)

+                return _gf_false;

+        if (local->flags & O_APPEND)

+                return _gf_true;

+        if (local->fd->flags & O_APPEND)

+                return _gf_true;

+        return _gf_false;

+}

+

 int

 __shard_get_delta_size_from_inode_ctx (shard_local_t *local, inode_t *inode,

                                        xlator_t *this)

@@ -3648,13 +3660,15 @@ __shard_get_delta_size_from_inode_ctx (shard_local_t *local, inode_t *inode,

         ctx = (shard_inode_ctx_t *) ctx_uint;

-        if (local->offset + local->total_size > ctx->stat.ia_size) {

+        if (shard_is_appending_write (local)) {

+                local->delta_size = local->total_size;

+        } else if (local->offset + local->total_size > ctx->stat.ia_size) {

                 local->delta_size = (local->offset + local->total_size) -

                                     ctx->stat.ia_size;

-                ctx->stat.ia_size += (local->delta_size);

         } else {

                 local->delta_size = 0;

         }

+        ctx->stat.ia_size += (local->delta_size);

         local->postbuf = ctx->stat;

         return 0;

@@ -3960,8 +3974,11 @@ shard_common_inode_write_post_mknod_handler (call_frame_t *frame,

 }

 int

-shard_common_inode_write_post_lookup_handler (call_frame_t *frame,

-                                              xlator_t *this)

+shard_mkdir_dot_shard (call_frame_t *frame, xlator_t *this,

+                       shard_post_resolve_fop_handler_t handler);

+int

+shard_common_inode_write_post_resolve_handler (call_frame_t *frame,

+                                               xlator_t *this)

 {

         shard_local_t *local = NULL;

@@ -3974,8 +3991,6 @@ shard_common_inode_write_post_lookup_handler (call_frame_t *frame,

                 return 0;

         }

-        local->postbuf = local->prebuf;

-

         if (local->call_count) {

                 shard_common_lookup_shards (frame, this,

                                             local->resolver_base_inode,

@@ -3988,12 +4003,11 @@ shard_common_inode_write_post_lookup_handler (call_frame_t *frame,

 }

 int

-shard_common_inode_write_post_resolve_handler (call_frame_t *frame,

-                                               xlator_t *this)

+shard_common_inode_write_post_lookup_handler (call_frame_t *frame,

+                                              xlator_t *this)

 {

-        shard_local_t *local = NULL;

-

-        local = frame->local;

+        shard_local_t *local = frame->local;

+        shard_priv_t  *priv  = this->private;

         if (local->op_ret < 0) {

                 shard_common_inode_write_failure_unwind (local->fop, frame,

@@ -4002,8 +4016,46 @@ shard_common_inode_write_post_resolve_handler (call_frame_t *frame,

                 return 0;

         }

-        shard_lookup_base_file (frame, this, &local->loc,

-                                shard_common_inode_write_post_lookup_handler);

+        local->postbuf = local->prebuf;

+

+        /*Adjust offset to EOF so that correct shard is chosen for append*/

+        if (shard_is_appending_write (local))

+                local->offset = local->prebuf.ia_size;

+

+        local->first_block = get_lowest_block (local->offset,

+                                               local->block_size);

+        local->last_block = get_highest_block (local->offset, local->total_size,

+                                               local->block_size);

+        local->num_blocks = local->last_block - local->first_block + 1;

+        local->inode_list = GF_CALLOC (local->num_blocks, sizeof (inode_t *),

+                                       gf_shard_mt_inode_list);

+        if (!local->inode_list) {

+                shard_common_inode_write_failure_unwind (local->fop, frame,

+                                                         -1, ENOMEM);

+                return 0;

+        }

+

+        gf_msg_trace (this->name, 0, "%s: gfid=%s first_block=%"PRIu32" "

+                      "last_block=%"PRIu32" num_blocks=%"PRIu32" offset=%"PRId64

+                      " total_size=%zu flags=%"PRId32"",

+                      gf_fop_list[local->fop],

+                      uuid_utoa (local->resolver_base_inode->gfid),

+                      local->first_block, local->last_block, local->num_blocks,

+                      local->offset, local->total_size, local->flags);

+

+        local->dot_shard_loc.inode = inode_find (this->itable,

+                                                 priv->dot_shard_gfid);

+

+        if (!local->dot_shard_loc.inode) {

+                /*change handler*/

+                shard_mkdir_dot_shard (frame, this,

+                                 shard_common_inode_write_post_resolve_handler);

+        } else {

+                /*change handler*/

+                local->post_res_handler =

+                                shard_common_inode_write_post_resolve_handler;

+                shard_refresh_dot_shard (frame, this);

+        }

         return 0;

 }

@@ -4706,9 +4758,6 @@ shard_common_inode_write_begin (call_frame_t *frame, xlator_t *this,

         int             i              = 0;

         uint64_t        block_size     = 0;

         shard_local_t  *local          = NULL;

-        shard_priv_t   *priv           = NULL;

-

-        priv = this->private;

         ret = shard_inode_ctx_get_block_size (fd->inode, this, &block_size);

         if (ret) {

@@ -4784,37 +4833,13 @@ shard_common_inode_write_begin (call_frame_t *frame, xlator_t *this,

                 local->iobref = iobref_ref (iobref);

         local->fd = fd_ref (fd);

         local->block_size = block_size;

-        local->first_block = get_lowest_block (offset, local->block_size);

-        local->last_block = get_highest_block (offset, local->total_size,

-                                               local->block_size);

-        local->num_blocks = local->last_block - local->first_block + 1;

         local->resolver_base_inode = local->fd->inode;

-        local->inode_list = GF_CALLOC (local->num_blocks, sizeof (inode_t *),

-                                       gf_shard_mt_inode_list);

-        if (!local->inode_list)

-                goto out;

         local->loc.inode = inode_ref (fd->inode);

         gf_uuid_copy (local->loc.gfid, fd->inode->gfid);

-        gf_msg_trace (this->name, 0, "%s: gfid=%s first_block=%"PRIu32" "

-                      "last_block=%"PRIu32" num_blocks=%"PRIu32" offset=%"PRId64""

-                      " total_size=%zu flags=%"PRId32"", gf_fop_list[fop],

-                      uuid_utoa (fd->inode->gfid), local->first_block,

-                      local->last_block, local->num_blocks, offset,

-                      local->total_size, local->flags);

-

-        local->dot_shard_loc.inode = inode_find (this->itable,

-                                                 priv->dot_shard_gfid);

-

-        if (!local->dot_shard_loc.inode) {

-                shard_mkdir_dot_shard (frame, this,

-                                 shard_common_inode_write_post_resolve_handler);

-        } else {

-                local->post_res_handler = shard_common_inode_write_post_resolve_handler;

-                shard_refresh_dot_shard (frame, this);

-        }

-

+        shard_lookup_base_file (frame, this, &local->loc,

+                                shard_common_inode_write_post_lookup_handler);

         return 0;

 out:

         shard_common_inode_write_failure_unwind (fop, frame, -1, ENOMEM);

-- 

1.8.3.1