From 2cb90b7798fa469f2d7d938ae88733eb1962d63d Mon Sep 17 00:00:00 2001

From: Xavi Hernandez <xhernandez@gmail.com>

Date: Fri, 9 Apr 2021 18:13:30 +0200

Subject: [PATCH 554/584] dht: fix rebalance of sparse files

Current implementation of rebalance for sparse files has a bug that,

in some cases, causes a read of 0 bytes from the source subvolume.

Posix xlator doesn't allow 0 byte reads and fails them with EINVAL,

which causes rebalance to abort the migration.

This patch implements a more robust way of finding data segments in

a sparse file that avoids 0 byte reads, allowing the file to be

migrated successfully.

Backport of:

> Upstream-patch: https://github.com/gluster/glusterfs/pull/2318

> Fixes: #2317

> Change-Id: Iff168dda2fb0f2edf716b21eb04cc2cc8ac3915c

> Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>

BUG: 1957641

Change-Id: Iff168dda2fb0f2edf716b21eb04cc2cc8ac3915c

Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/244551

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>

---

 tests/bugs/distribute/issue-2317.t      |  29 ++++++++

 tests/volume.rc                         |   4 ++

 xlators/cluster/dht/src/dht-rebalance.c | 116 +++++++++++++++++---------------

 3 files changed, 93 insertions(+), 56 deletions(-)

 create mode 100755 tests/bugs/distribute/issue-2317.t

diff --git a/tests/bugs/distribute/issue-2317.t b/tests/bugs/distribute/issue-2317.t

new file mode 100755

index 0000000..e29d003

--- /dev/null

+++ b/tests/bugs/distribute/issue-2317.t

@@ -0,0 +1,29 @@

+#!/bin/bash

+

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

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

+

+TESTS_EXPECTED_IN_LOOP=126

+

+cleanup

+

+TEST glusterd

+TEST ${CLI} volume create ${V0} replica 3 ${H0}:/$B0/${V0}_{0..2}

+TEST ${CLI} volume start ${V0}

+

+TEST ${GFS} --volfile-server ${H0} --volfile-id ${V0} ${M0}

+

+# Create several files to make sure that at least some of them should be

+# migrated by rebalance.

+for i in {0..63}; do

+    TEST dd if=/dev/urandom of=${M0}/file.${i} bs=4k count=1

+    TEST dd if=/dev/urandom of=${M0}/file.${i} bs=4k count=1 seek=128

+done

+

+TEST ${CLI} volume add-brick ${V0} ${H0}:${B0}/${V0}_{3..5}

+TEST ${CLI} volume rebalance ${V0} start force

+EXPECT_WITHIN ${REBALANCE_TIMEOUT} "completed" rebalance_status_field "${V0}"

+

+EXPECT "^0$" rebalance_failed_field "${V0}"

+

+cleanup

diff --git a/tests/volume.rc b/tests/volume.rc

index 9a002d9..f5dd0b1 100644

--- a/tests/volume.rc

+++ b/tests/volume.rc

@@ -75,6 +75,10 @@ function rebalance_status_field {

         $CLI volume rebalance $1 status | awk '{print $7}' | sed -n 3p

 }

+function rebalance_failed_field {

+        $CLI volume rebalance $1 status | awk '{print $5}' | sed -n 3p

+}

+

 function fix-layout_status_field {

         #The fix-layout status can be up to 3 words, (ex:'fix-layout in progress'), hence the awk-print $2 thru $4.

         #But if the status is less than 3 words, it also prints the next field i.e the run_time_in_secs.(ex:'completed 3.00').

diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c

index 072896d..eab7558 100644

--- a/xlators/cluster/dht/src/dht-rebalance.c

+++ b/xlators/cluster/dht/src/dht-rebalance.c

@@ -1024,6 +1024,46 @@ out:

     return ret;

 }

+static int32_t

+dht_rebalance_sparse_segment(xlator_t *subvol, fd_t *fd, off_t *offset,

+                             size_t *size)

+{

+    off_t hole;

+    int32_t ret;

+

+    do {

+        ret = syncop_seek(subvol, fd, *offset, GF_SEEK_DATA, NULL, offset);

+        if (ret >= 0) {

+            /* Starting at the offset of the last data segment, find the

+             * next hole. After a data segment there should always be a

+             * hole, since EOF is considered a hole. */

+            ret = syncop_seek(subvol, fd, *offset, GF_SEEK_HOLE, NULL, &hole);

+        }

+

+        if (ret < 0) {

+            if (ret == -ENXIO) {

+                /* This can happen if there are no more data segments (i.e.

+                 * the offset is at EOF), or there was a data segment but the

+                 * file has been truncated to a smaller size between both

+                 * seek requests. In both cases we are done. The file doesn't

+                 * contain more data. */

+                ret = 0;

+            }

+            return ret;

+        }

+

+        /* It could happen that at the same offset we detected data in the

+         * first seek, there could be a hole in the second seek if user is

+         * modifying the file concurrently. In this case we need to find a

+         * new data segment to migrate. */

+    } while (hole <= *offset);

+

+    /* Calculate the total size of the current data block */

+    *size = hole - *offset;

+

+    return 1;

+}

+

 static int

 __dht_rebalance_migrate_data(xlator_t *this, gf_defrag_info_t *defrag,

                              xlator_t *from, xlator_t *to, fd_t *src, fd_t *dst,

@@ -1032,8 +1072,6 @@ __dht_rebalance_migrate_data(xlator_t *this, gf_defrag_info_t *defrag,

     int ret = 0;

     int count = 0;

     off_t offset = 0;

-    off_t data_offset = 0;

-    off_t hole_offset = 0;

     struct iovec *vector = NULL;

     struct iobref *iobref = NULL;

     uint64_t total = 0;

@@ -1048,71 +1086,36 @@ __dht_rebalance_migrate_data(xlator_t *this, gf_defrag_info_t *defrag,

     while (total < ia_size) {

         /* This is a regular file - read it sequentially */

         if (!hole_exists) {

-            read_size = (((ia_size - total) > DHT_REBALANCE_BLKSIZE)

-                             ? DHT_REBALANCE_BLKSIZE

-                             : (ia_size - total));

+            data_block_size = ia_size - total;

         } else {

             /* This is a sparse file - read only the data segments in the file

              */

             /* If the previous data block is fully copied, find the next data

-             * segment

-             * starting at the offset of the last read and written byte,  */

+             * segment starting at the offset of the last read and written

+             * byte. */

             if (data_block_size <= 0) {

-                ret = syncop_seek(from, src, offset, GF_SEEK_DATA, NULL,

-                                  &data_offset);

-                if (ret) {

-                    if (ret == -ENXIO)

-                        ret = 0; /* No more data segments */

-                    else

-                        *fop_errno = -ret; /* Error occurred */

-

+                ret = dht_rebalance_sparse_segment(from, src, &offset,

+                                                   &data_block_size);

+                if (ret <= 0) {

+                    *fop_errno = -ret;

                     break;

                 }

-

-                /* If the position of the current data segment is greater than

-                 * the position of the next hole, find the next hole in order to

-                 * calculate the length of the new data segment */

-                if (data_offset > hole_offset) {

-                    /* Starting at the offset of the last data segment, find the

-                     * next hole */

-                    ret = syncop_seek(from, src, data_offset, GF_SEEK_HOLE,

-                                      NULL, &hole_offset);

-                    if (ret) {

-                        /* If an error occurred here it's a real error because

-                         * if the seek for a data segment was successful then

-                         * necessarily another hole must exist (EOF is a hole)

-                         */

-                        *fop_errno = -ret;

-                        break;

-                    }

-

-                    /* Calculate the total size of the current data block */

-                    data_block_size = hole_offset - data_offset;

-                }

-            } else {

-                /* There is still data in the current segment, move the

-                 * data_offset to the position of the last written byte */

-                data_offset = offset;

             }

-

-            /* Calculate how much data needs to be read and written. If the data

-             * segment's length is bigger than DHT_REBALANCE_BLKSIZE, read and

-             * write DHT_REBALANCE_BLKSIZE data length and the rest in the

-             * next iteration(s) */

-            read_size = ((data_block_size > DHT_REBALANCE_BLKSIZE)

-                             ? DHT_REBALANCE_BLKSIZE

-                             : data_block_size);

-

-            /* Calculate the remaining size of the data block - maybe there's no

-             * need to seek for data in the next iteration */

-            data_block_size -= read_size;

-

-            /* Set offset to the offset of the data segment so read and write

-             * will have the correct position */

-            offset = data_offset;

         }

+        /* Calculate how much data needs to be read and written. If the data

+         * segment's length is bigger than DHT_REBALANCE_BLKSIZE, read and

+         * write DHT_REBALANCE_BLKSIZE data length and the rest in the

+         * next iteration(s) */

+        read_size = ((data_block_size > DHT_REBALANCE_BLKSIZE)

+                         ? DHT_REBALANCE_BLKSIZE

+                         : data_block_size);

+

+        /* Calculate the remaining size of the data block - maybe there's no

+         * need to seek for data in the next iteration */

+        data_block_size -= read_size;

+

         ret = syncop_readv(from, src, read_size, offset, 0, &vector, &count,

                            &iobref, NULL, NULL, NULL);

@@ -1177,6 +1180,7 @@ __dht_rebalance_migrate_data(xlator_t *this, gf_defrag_info_t *defrag,

         iobref = NULL;

         vector = NULL;

     }

+

     if (iobref)

         iobref_unref(iobref);

     GF_FREE(vector);

-- 

1.8.3.1