From 1a8b001a121ada4d3d338b52b312896f1790f2bb Mon Sep 17 00:00:00 2001 From: Vinayak hariharmath <65405035+VHariharmath-rh@users.noreply.github.com> Date: Mon, 11 Jan 2021 12:34:55 +0530 Subject: [PATCH 570/584] features/shard: avoid repeatative calls to gf_uuid_unparse() The issue is shard_make_block_abspath() calls gf_uuid_unparse() every time while constructing shard path. The gfid can be parsed and saved once and passed while constructing the path. Thus we can avoid calling gf_uuid_unparse(). Backport of: > Upstream-patch: https://github.com/gluster/glusterfs/pull/1689 > Fixes: #1423 > Change-Id: Ia26fbd5f09e812bbad9e5715242f14143c013c9c > Signed-off-by: Vinayakswami Hariharmath vharihar@redhat.com BUG: 1925425 Change-Id: Ia26fbd5f09e812bbad9e5715242f14143c013c9c Signed-off-by: Vinayakswami Hariharmath vharihar@redhat.com Reviewed-on: https://code.engineering.redhat.com/gerrit/c/rhs-glusterfs/+/244964 Tested-by: RHGS Build Bot Reviewed-by: Sunil Kumar Heggodu Gopala Acharya --- tests/bugs/shard/issue-1425.t | 9 ++- xlators/features/shard/src/shard.c | 119 ++++++++++++++++++------------------- 2 files changed, 65 insertions(+), 63 deletions(-) diff --git a/tests/bugs/shard/issue-1425.t b/tests/bugs/shard/issue-1425.t index bbe82c0..8b77705 100644 --- a/tests/bugs/shard/issue-1425.t +++ b/tests/bugs/shard/issue-1425.t @@ -21,7 +21,13 @@ TEST $CLI volume profile $V0 start TEST $GFS --volfile-id=$V0 --volfile-server=$H0 $M0 +$CLI volume profile $V0 info clear + TEST fallocate -l 20M $M0/foo + +# There should be 1+4 shards and we expect 4 lookups less than on the build without this patch +EXPECT "5" echo `$CLI volume profile $V0 info incremental | grep -w LOOKUP | awk '{print $8}'` + gfid_new=$(get_gfid_string $M0/foo) # Check for the base shard @@ -31,9 +37,6 @@ TEST stat $B0/${V0}0/foo # There should be 4 associated shards EXPECT_WITHIN $FILE_COUNT_TIME 4 get_file_count $B0/${V0}0/.shard/$gfid_new -# There should be 1+4 shards and we expect 4 lookups less than on the build without this patch -EXPECT "21" echo `$CLI volume profile $V0 info incremental | grep -w LOOKUP | awk '{print $8}'` - # Delete the base shard and check shards get cleaned up TEST unlink $M0/foo diff --git a/xlators/features/shard/src/shard.c b/xlators/features/shard/src/shard.c index a6ad1b8..d1d7d7a 100644 --- a/xlators/features/shard/src/shard.c +++ b/xlators/features/shard/src/shard.c @@ -16,6 +16,8 @@ #include #include +#define SHARD_PATH_MAX (sizeof(GF_SHARD_DIR) + GF_UUID_BUF_SIZE + 16) + static gf_boolean_t __is_shard_dir(uuid_t gfid) { @@ -49,15 +51,19 @@ shard_make_block_bname(int block_num, uuid_t gfid, char *buf, size_t len) snprintf(buf, len, "%s.%d", gfid_str, block_num); } -void -shard_make_block_abspath(int block_num, uuid_t gfid, char *filepath, size_t len) +static int +shard_make_base_path(char *path, uuid_t gfid) { - char gfid_str[GF_UUID_BUF_SIZE] = { - 0, - }; + strcpy(path, "/" GF_SHARD_DIR "/"); + uuid_utoa_r(gfid, path + sizeof(GF_SHARD_DIR) + 1); + return (sizeof(GF_SHARD_DIR) + GF_UUID_BUF_SIZE); +} - gf_uuid_unparse(gfid, gfid_str); - snprintf(filepath, len, "/%s/%s.%d", GF_SHARD_DIR, gfid_str, block_num); +static inline void +shard_append_index(char *path, int path_size, int prefix_len, + int shard_idx_iter) +{ + snprintf(path + prefix_len, path_size - prefix_len, ".%d", shard_idx_iter); } int @@ -1004,9 +1010,8 @@ shard_common_resolve_shards(call_frame_t *frame, xlator_t *this, { int i = -1; uint32_t shard_idx_iter = 0; - char path[PATH_MAX] = { - 0, - }; + int prefix_len = 0; + char path[SHARD_PATH_MAX]; uuid_t gfid = { 0, }; @@ -1055,6 +1060,9 @@ shard_common_resolve_shards(call_frame_t *frame, xlator_t *this, else gf_uuid_copy(gfid, local->base_gfid); + /* Build base shard path before appending index of the shard */ + prefix_len = shard_make_base_path(path, gfid); + while (shard_idx_iter <= resolve_count) { i++; if (shard_idx_iter == 0) { @@ -1062,16 +1070,13 @@ shard_common_resolve_shards(call_frame_t *frame, xlator_t *this, shard_idx_iter++; continue; } - - shard_make_block_abspath(shard_idx_iter, gfid, path, sizeof(path)); - + shard_append_index(path, SHARD_PATH_MAX, prefix_len, shard_idx_iter); inode = NULL; inode = inode_resolve(this->itable, path); if (inode) { gf_msg_debug(this->name, 0, - "Shard %d already " - "present. gfid=%s. Saving inode for future.", - shard_idx_iter, uuid_utoa(inode->gfid)); + "Shard %s already present. Saving inode for future.", + path); local->inode_list[i] = inode; /* Let the ref on the inodes that are already present * in inode table still be held so that they don't get @@ -2153,9 +2158,8 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode) int call_count = 0; uint32_t cur_block = 0; uint32_t last_block = 0; - char path[PATH_MAX] = { - 0, - }; + int prefix_len = 0; + char path[SHARD_PATH_MAX]; char *bname = NULL; loc_t loc = { 0, @@ -2216,6 +2220,10 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode) return 0; } + /* Build base shard path before appending index of the shard */ + prefix_len = shard_make_base_path(path, inode->gfid); + bname = path + sizeof(GF_SHARD_DIR) + 1; + SHARD_SET_ROOT_FS_ID(frame, local); while (cur_block <= last_block) { if (!local->inode_list[i]) { @@ -2229,15 +2237,12 @@ shard_truncate_htol(call_frame_t *frame, xlator_t *this, inode_t *inode) goto next; } - shard_make_block_abspath(cur_block, inode->gfid, path, sizeof(path)); - bname = strrchr(path, '/') + 1; + shard_append_index(path, SHARD_PATH_MAX, prefix_len, cur_block); loc.parent = inode_ref(priv->dot_shard_inode); ret = inode_path(loc.parent, bname, (char **)&(loc.path)); if (ret < 0) { gf_msg(this->name, GF_LOG_ERROR, 0, SHARD_MSG_INODE_PATH_FAILED, - "Inode path failed" - " on %s. Base file gfid = %s", - bname, uuid_utoa(inode->gfid)); + "Inode path failed on %s.", bname); local->op_ret = -1; local->op_errno = ENOMEM; loc_wipe(&loc); @@ -2465,13 +2470,8 @@ shard_common_lookup_shards(call_frame_t *frame, xlator_t *this, inode_t *inode, int call_count = 0; int32_t shard_idx_iter = 0; int lookup_count = 0; - char path[PATH_MAX] = { - 0, - }; + char path[SHARD_PATH_MAX]; char *bname = NULL; - uuid_t gfid = { - 0, - }; loc_t loc = { 0, }; @@ -2489,10 +2489,16 @@ shard_common_lookup_shards(call_frame_t *frame, xlator_t *this, inode_t *inode, if (local->lookup_shards_barriered) local->barrier.waitfor = local->call_count; + /* Build base shard path before appending index of the shard */ + strcpy(path, "/" GF_SHARD_DIR "/"); + if (inode) - gf_uuid_copy(gfid, inode->gfid); + uuid_utoa_r(inode->gfid, path + sizeof(GF_SHARD_DIR) + 1); else - gf_uuid_copy(gfid, local->base_gfid); + uuid_utoa_r(local->base_gfid, path + sizeof(GF_SHARD_DIR) + 1); + + int prefix_len = sizeof(GF_SHARD_DIR) + GF_UUID_BUF_SIZE; + bname = path + sizeof(GF_SHARD_DIR) + 1; while (shard_idx_iter <= lookup_count) { if (local->inode_list[i]) { @@ -2508,18 +2514,14 @@ shard_common_lookup_shards(call_frame_t *frame, xlator_t *this, inode_t *inode, goto next; } - shard_make_block_abspath(shard_idx_iter, gfid, path, sizeof(path)); - - bname = strrchr(path, '/') + 1; + shard_append_index(path, SHARD_PATH_MAX, prefix_len, shard_idx_iter); loc.inode = inode_new(this->itable); loc.parent = inode_ref(priv->dot_shard_inode); gf_uuid_copy(loc.pargfid, priv->dot_shard_gfid); ret = inode_path(loc.parent, bname, (char **)&(loc.path)); if (ret < 0 || !(loc.inode)) { gf_msg(this->name, GF_LOG_ERROR, 0, SHARD_MSG_INODE_PATH_FAILED, - "Inode path failed" - " on %s, base file gfid = %s", - bname, uuid_utoa(gfid)); + "Inode path failed on %s", bname); local->op_ret = -1; local->op_errno = ENOMEM; loc_wipe(&loc); @@ -3168,12 +3170,7 @@ shard_unlink_shards_do(call_frame_t *frame, xlator_t *this, inode_t *inode) uint32_t cur_block = 0; uint32_t cur_block_idx = 0; /*this is idx into inode_list[] array */ char *bname = NULL; - char path[PATH_MAX] = { - 0, - }; - uuid_t gfid = { - 0, - }; + char path[SHARD_PATH_MAX]; loc_t loc = { 0, }; @@ -3184,10 +3181,16 @@ shard_unlink_shards_do(call_frame_t *frame, xlator_t *this, inode_t *inode) priv = this->private; local = frame->local; + /* Build base shard path before appending index of the shard */ + strcpy(path, "/" GF_SHARD_DIR "/"); + if (inode) - gf_uuid_copy(gfid, inode->gfid); + uuid_utoa_r(inode->gfid, path + sizeof(GF_SHARD_DIR) + 1); else - gf_uuid_copy(gfid, local->base_gfid); + uuid_utoa_r(local->base_gfid, path + sizeof(GF_SHARD_DIR) + 1); + + int prefix_len = sizeof(GF_SHARD_DIR) + GF_UUID_BUF_SIZE; + bname = path + sizeof(GF_SHARD_DIR) + 1; for (i = 0; i < local->num_blocks; i++) { if (!local->inode_list[i]) @@ -3203,7 +3206,7 @@ shard_unlink_shards_do(call_frame_t *frame, xlator_t *this, inode_t *inode) gf_msg_debug(this->name, 0, "All shards that need to be " "unlinked are non-existent: %s", - uuid_utoa(gfid)); + path); return 0; } @@ -3221,15 +3224,12 @@ shard_unlink_shards_do(call_frame_t *frame, xlator_t *this, inode_t *inode) goto next; } - shard_make_block_abspath(cur_block, gfid, path, sizeof(path)); - bname = strrchr(path, '/') + 1; + shard_append_index(path, SHARD_PATH_MAX, prefix_len, cur_block); loc.parent = inode_ref(priv->dot_shard_inode); ret = inode_path(loc.parent, bname, (char **)&(loc.path)); if (ret < 0) { gf_msg(this->name, GF_LOG_ERROR, 0, SHARD_MSG_INODE_PATH_FAILED, - "Inode path failed" - " on %s, base file gfid = %s", - bname, uuid_utoa(gfid)); + "Inode path failed on %s", bname); local->op_ret = -1; local->op_errno = ENOMEM; loc_wipe(&loc); @@ -4971,9 +4971,8 @@ shard_common_resume_mknod(call_frame_t *frame, xlator_t *this, int last_block = 0; int ret = 0; int call_count = 0; - char path[PATH_MAX] = { - 0, - }; + int prefix_len = 0; + char path[SHARD_PATH_MAX]; mode_t mode = 0; char *bname = NULL; shard_priv_t *priv = NULL; @@ -4996,6 +4995,10 @@ shard_common_resume_mknod(call_frame_t *frame, xlator_t *this, call_count = local->call_count = local->create_count; local->post_mknod_handler = post_mknod_handler; + /* Build base shard path before appending index of the shard */ + prefix_len = shard_make_base_path(path, fd->inode->gfid); + bname = path + sizeof(GF_SHARD_DIR) + 1; + SHARD_SET_ROOT_FS_ID(frame, local); ret = shard_inode_ctx_get_all(fd->inode, this, &ctx_tmp); @@ -5022,10 +5025,7 @@ shard_common_resume_mknod(call_frame_t *frame, xlator_t *this, -1, ENOMEM, NULL, NULL, NULL, NULL, NULL); goto next; } - - shard_make_block_abspath(shard_idx_iter, fd->inode->gfid, path, - sizeof(path)); - + shard_append_index(path, SHARD_PATH_MAX, prefix_len, shard_idx_iter); xattr_req = shard_create_gfid_dict(local->xattr_req); if (!xattr_req) { local->op_ret = -1; @@ -5036,7 +5036,6 @@ shard_common_resume_mknod(call_frame_t *frame, xlator_t *this, goto next; } - bname = strrchr(path, '/') + 1; loc.inode = inode_new(this->itable); loc.parent = inode_ref(priv->dot_shard_inode); ret = inode_path(loc.parent, bname, (char **)&(loc.path)); -- 1.8.3.1