path: root/0189-features-shard-Fix-block-count-accounting-upon-trunc.patch

From 369c5772a722b6e346ec8b41f992112785366778 Mon Sep 17 00:00:00 2001
From: Krutika Dhananjay <kdhananj@redhat.com>
Date: Wed, 8 May 2019 13:00:51 +0530
Subject: [PATCH 189/192] features/shard: Fix block-count accounting upon
 truncate to lower size

    > Upstream: https://review.gluster.org/22681
    > BUG: 1705884
    > Change-Id: I9128a192e9bf8c3c3a959e96b7400879d03d7c53

The way delta_blocks is computed in shard is incorrect, when a file
is truncated to a lower size. The accounting only considers change
in size of the last of the truncated shards.

FIX:

Get the block-count of each shard just before an unlink at posix in
xdata.  Their summation plus the change in size of last shard
(from an actual truncate) is used to compute delta_blocks which is
used in the xattrop for size update.

Change-Id: I9128a192e9bf8c3c3a959e96b7400879d03d7c53
updates: bz#1668001
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/173477
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
 libglusterfs/src/glusterfs/glusterfs.h      |  2 +
 tests/bugs/shard/bug-1705884.t              | 32 +++++++++++++++
 xlators/features/shard/src/shard.c          | 60 +++++++++++++++++++++++------
 xlators/features/shard/src/shard.h          |  2 +-
 xlators/storage/posix/src/posix-entry-ops.c |  9 +++++
 5 files changed, 92 insertions(+), 13 deletions(-)
 create mode 100644 tests/bugs/shard/bug-1705884.t

diff --git a/libglusterfs/src/glusterfs/glusterfs.h b/libglusterfs/src/glusterfs/glusterfs.h
index 516b497..9ec2365 100644
--- a/libglusterfs/src/glusterfs/glusterfs.h
+++ b/libglusterfs/src/glusterfs/glusterfs.h
@@ -328,6 +328,8 @@ enum gf_internal_fop_indicator {
 #define GF_RESPONSE_LINK_COUNT_XDATA "gf_response_link_count"
 #define GF_REQUEST_LINK_COUNT_XDATA "gf_request_link_count"
 
+#define GF_GET_FILE_BLOCK_COUNT "gf_get_file_block_count"
+
 #define CTR_ATTACH_TIER_LOOKUP "ctr_attach_tier_lookup"
 
 #define CLIENT_CMD_CONNECT "trusted.glusterfs.client-connect"
diff --git a/tests/bugs/shard/bug-1705884.t b/tests/bugs/shard/bug-1705884.t
new file mode 100644
index 0000000..f6e5037
--- /dev/null
+++ b/tests/bugs/shard/bug-1705884.t
@@ -0,0 +1,32 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+. $(dirname $0)/../../fallocate.rc
+
+cleanup
+
+require_fallocate -l 1m $M0/file
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2}
+TEST $CLI volume set $V0 features.shard on
+TEST $CLI volume set $V0 performance.write-behind off
+TEST $CLI volume set $V0 performance.stat-prefetch off
+TEST $CLI volume start $V0
+
+TEST $GFS --volfile-id=$V0 --volfile-server=$H0 $M0
+
+TEST fallocate -l 200M $M0/foo
+EXPECT `echo "$(( ( 200 * 1024 * 1024 ) / 512 ))"`  stat -c %b $M0/foo
+TEST truncate -s 0 $M0/foo
+EXPECT "0" stat -c %b $M0/foo
+TEST fallocate -l 100M $M0/foo
+EXPECT `echo "$(( ( 100 * 1024 * 1024 ) / 512 ))"`  stat -c %b $M0/foo
+
+EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0
+TEST $CLI volume stop $V0
+TEST $CLI volume delete $V0
+
+cleanup
diff --git a/xlators/features/shard/src/shard.c b/xlators/features/shard/src/shard.c
index c1799ad..b248767 100644
--- a/xlators/features/shard/src/shard.c
+++ b/xlators/features/shard/src/shard.c
@@ -1135,6 +1135,7 @@ shard_update_file_size(call_frame_t *frame, xlator_t *this, fd_t *fd,
 {
     int ret = -1;
     int64_t *size_attr = NULL;
+    int64_t delta_blocks = 0;
     inode_t *inode = NULL;
     shard_local_t *local = NULL;
     dict_t *xattr_req = NULL;
@@ -1156,13 +1157,13 @@ shard_update_file_size(call_frame_t *frame, xlator_t *this, fd_t *fd,
 
     /* If both size and block count have not changed, then skip the xattrop.
      */
-    if ((local->delta_size + local->hole_size == 0) &&
-        (local->delta_blocks == 0)) {
+    delta_blocks = GF_ATOMIC_GET(local->delta_blocks);
+    if ((local->delta_size + local->hole_size == 0) && (delta_blocks == 0)) {
         goto out;
     }
 
     ret = shard_set_size_attrs(local->delta_size + local->hole_size,
-                               local->delta_blocks, &size_attr);
+                               delta_blocks, &size_attr);
     if (ret) {
         gf_msg(this->name, GF_LOG_ERROR, 0, SHARD_MSG_SIZE_SET_FAILED,
                "Failed to set size attrs for %s", uuid_utoa(inode->gfid));
@@ -1947,6 +1948,7 @@ shard_truncate_last_shard_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
                               dict_t *xdata)
 {
     inode_t *inode = NULL;
+    int64_t delta_blocks = 0;
     shard_local_t *local = NULL;
 
     local = frame->local;
@@ -1967,14 +1969,15 @@ shard_truncate_last_shard_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
     }
 
     local->postbuf.ia_size = local->offset;
-    local->postbuf.ia_blocks -= (prebuf->ia_blocks - postbuf->ia_blocks);
     /* Let the delta be negative. We want xattrop to do subtraction */
     local->delta_size = local->postbuf.ia_size - local->prebuf.ia_size;
-    local->delta_blocks = postbuf->ia_blocks - prebuf->ia_blocks;
+    delta_blocks = GF_ATOMIC_ADD(local->delta_blocks,
+                                 postbuf->ia_blocks - prebuf->ia_blocks);
+    GF_ASSERT(delta_blocks <= 0);
+    local->postbuf.ia_blocks += delta_blocks;
     local->hole_size = 0;
 
-    shard_inode_ctx_set(inode, this, postbuf, 0, SHARD_MASK_TIMES);
-
+    shard_inode_ctx_set(inode, this, &local->postbuf, 0, SHARD_MASK_TIMES);
     shard_update_file_size(frame, this, NULL, &local->loc,
                            shard_post_update_size_truncate_handler);
     return 0;
@@ -2034,8 +2037,10 @@ shard_truncate_htol_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
                         struct iatt *preparent, struct iatt *postparent,
                         dict_t *xdata)
 {
+    int ret = 0;
     int call_count = 0;
     int shard_block_num = (long)cookie;
+    uint64_t block_count = 0;
     shard_local_t *local = NULL;
 
     local = frame->local;
@@ -2045,6 +2050,16 @@ shard_truncate_htol_cbk(call_frame_t *frame, void *cookie, xlator_t *this,
         local->op_errno = op_errno;
         goto done;
     }
+    ret = dict_get_uint64(xdata, GF_GET_FILE_BLOCK_COUNT, &block_count);
+    if (!ret) {
+        GF_ATOMIC_SUB(local->delta_blocks, block_count);
+    } else {
+        /* dict_get failed possibly due to a heterogeneous cluster? */
+        gf_msg(this->name, GF_LOG_WARNING, 0, SHARD_MSG_DICT_OP_FAILED,
+               "Failed to get key %s from dict during truncate of gfid %s",
+               GF_GET_FILE_BLOCK_COUNT,
+               uuid_utoa(local->resolver_base_inode->gfid));
+    }
 
     shard_unlink_block_inode(local, shard_block_num);
 done:
@@ -2074,6 +2089,7 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode)
     gf_boolean_t wind_failed = _gf_false;
     shard_local_t *local = NULL;
     shard_priv_t *priv = NULL;
+    dict_t *xdata_req = NULL;
 
     local = frame->local;
     priv = this->private;
@@ -2101,7 +2117,7 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode)
         local->postbuf.ia_size = local->offset;
         local->postbuf.ia_blocks = local->prebuf.ia_blocks;
         local->delta_size = local->postbuf.ia_size - local->prebuf.ia_size;
-        local->delta_blocks = 0;
+        GF_ATOMIC_INIT(local->delta_blocks, 0);
         local->hole_size = 0;
         shard_update_file_size(frame, this, local->fd, &local->loc,
                                shard_post_update_size_truncate_handler);
@@ -2110,6 +2126,21 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode)
 
     local->call_count = call_count;
     i = 1;
+    xdata_req = dict_new();
+    if (!xdata_req) {
+        shard_common_failure_unwind(local->fop, frame, -1, ENOMEM);
+        return 0;
+    }
+    ret = dict_set_uint64(xdata_req, GF_GET_FILE_BLOCK_COUNT, 8 * 8);
+    if (ret) {
+        gf_msg(this->name, GF_LOG_WARNING, 0, SHARD_MSG_DICT_OP_FAILED,
+               "Failed to set key %s into dict during truncate of %s",
+               GF_GET_FILE_BLOCK_COUNT,
+               uuid_utoa(local->resolver_base_inode->gfid));
+        dict_unref(xdata_req);
+        shard_common_failure_unwind(local->fop, frame, -1, ENOMEM);
+        return 0;
+    }
 
     SHARD_SET_ROOT_FS_ID(frame, local);
     while (cur_block <= last_block) {
@@ -2148,7 +2179,7 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode)
 
         STACK_WIND_COOKIE(frame, shard_truncate_htol_cbk,
                           (void *)(long)cur_block, FIRST_CHILD(this),
-                          FIRST_CHILD(this)->fops->unlink, &loc, 0, NULL);
+                          FIRST_CHILD(this)->fops->unlink, &loc, 0, xdata_req);
         loc_wipe(&loc);
     next:
         i++;
@@ -2156,6 +2187,7 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode)
         if (!--call_count)
             break;
     }
+    dict_unref(xdata_req);
     return 0;
 }
 
@@ -2608,7 +2640,7 @@ shard_post_lookup_truncate_handler(call_frame_t *frame, xlator_t *this)
          */
         local->hole_size = local->offset - local->prebuf.ia_size;
         local->delta_size = 0;
-        local->delta_blocks = 0;
+        GF_ATOMIC_INIT(local->delta_blocks, 0);
         local->postbuf.ia_size = local->offset;
         tmp_stbuf.ia_size = local->offset;
         shard_inode_ctx_set(local->loc.inode, this, &tmp_stbuf, 0,
@@ -2624,7 +2656,7 @@ shard_post_lookup_truncate_handler(call_frame_t *frame, xlator_t *this)
          */
         local->hole_size = 0;
         local->delta_size = (local->offset - local->prebuf.ia_size);
-        local->delta_blocks = 0;
+        GF_ATOMIC_INIT(local->delta_blocks, 0);
         tmp_stbuf.ia_size = local->offset;
         shard_inode_ctx_set(local->loc.inode, this, &tmp_stbuf, 0,
                             SHARD_INODE_WRITE_MASK);
@@ -2680,6 +2712,7 @@ shard_truncate(call_frame_t *frame, xlator_t *this, loc_t *loc, off_t offset,
     if (!local->xattr_req)
         goto err;
     local->resolver_base_inode = loc->inode;
+    GF_ATOMIC_INIT(local->delta_blocks, 0);
 
     shard_lookup_base_file(frame, this, &local->loc,
                            shard_post_lookup_truncate_handler);
@@ -2735,6 +2768,7 @@ shard_ftruncate(call_frame_t *frame, xlator_t *this, fd_t *fd, off_t offset,
     local->loc.inode = inode_ref(fd->inode);
     gf_uuid_copy(local->loc.gfid, fd->inode->gfid);
     local->resolver_base_inode = fd->inode;
+    GF_ATOMIC_INIT(local->delta_blocks, 0);
 
     shard_lookup_base_file(frame, this, &local->loc,
                            shard_post_lookup_truncate_handler);
@@ -5295,7 +5329,8 @@ shard_common_inode_write_do_cbk(call_frame_t *frame, void *cookie,
             local->op_errno = op_errno;
         } else {
             local->written_size += op_ret;
-            local->delta_blocks += (post->ia_blocks - pre->ia_blocks);
+            GF_ATOMIC_ADD(local->delta_blocks,
+                          post->ia_blocks - pre->ia_blocks);
             local->delta_size += (post->ia_size - pre->ia_size);
             shard_inode_ctx_set(local->fd->inode, this, post, 0,
                                 SHARD_MASK_TIMES);
@@ -6599,6 +6634,7 @@ shard_common_inode_write_begin(call_frame_t *frame, xlator_t *this,
     local->fd = fd_ref(fd);
     local->block_size = block_size;
     local->resolver_base_inode = local->fd->inode;
+    GF_ATOMIC_INIT(local->delta_blocks, 0);
 
     local->loc.inode = inode_ref(fd->inode);
     gf_uuid_copy(local->loc.gfid, fd->inode->gfid);
diff --git a/xlators/features/shard/src/shard.h b/xlators/features/shard/src/shard.h
index cd6a663..04abd62 100644
--- a/xlators/features/shard/src/shard.h
+++ b/xlators/features/shard/src/shard.h
@@ -275,7 +275,7 @@ typedef struct shard_local {
     size_t req_size;
     size_t readdir_size;
     int64_t delta_size;
-    int64_t delta_blocks;
+    gf_atomic_t delta_blocks;
     loc_t loc;
     loc_t dot_shard_loc;
     loc_t dot_shard_rm_loc;
diff --git a/xlators/storage/posix/src/posix-entry-ops.c b/xlators/storage/posix/src/posix-entry-ops.c
index b24a052..34ee2b8 100644
--- a/xlators/storage/posix/src/posix-entry-ops.c
+++ b/xlators/storage/posix/src/posix-entry-ops.c
@@ -1071,6 +1071,7 @@ posix_unlink(call_frame_t *frame, xlator_t *this, loc_t *loc, int xflag,
     char *real_path = NULL;
     char *par_path = NULL;
     int32_t fd = -1;
+    int ret = -1;
     struct iatt stbuf = {
         0,
     };
@@ -1235,6 +1236,14 @@ posix_unlink(call_frame_t *frame, xlator_t *this, loc_t *loc, int xflag,
         goto out;
     }
 
+    if (xdata && dict_get(xdata, GF_GET_FILE_BLOCK_COUNT)) {
+        ret = dict_set_uint64(unwind_dict, GF_GET_FILE_BLOCK_COUNT,
+                              stbuf.ia_blocks);
+        if (ret)
+            gf_msg(this->name, GF_LOG_WARNING, 0, P_MSG_SET_XDATA_FAIL,
+                   "Failed to set %s in rsp dict", GF_GET_FILE_BLOCK_COUNT);
+    }
+
     if (xdata && dict_get(xdata, GET_LINK_COUNT))
         get_link_count = _gf_true;
     op_ret = posix_unlink_gfid_handle_and_entry(frame, this, real_path, &stbuf,
-- 
1.8.3.1