glusterd:Reducing file operations when writing options into volfile.

Issue: The options to be written into the volfile are in form of
key-value pairs and the current approach taken to write them into
a file is to invoke the write syscall for each key-value pair.
This implies an increased number of system calls.

Code Changes:
1. Addition of a structure, glusterd_volinfo_data_store_t in
glusterd-store.h, containing a character buffer, a pointer to
gf_store_handle_t, the current length of data in the buffer as
well as a flag for checking key while storing in the buffer.
This is used for passing the required file descriptor as well
having a character buffer for storing multiple options before
being written into a file.

2. Modification of function, _storeopts in glusterd-store.c, which
now invokes the gf_store_save_items when buffer is to be emptied
into the volfile before further write into it. Also, it has
replaced the function _storeslaves, _gd_store_rebalance_dict
and _store_global_opts.

3. Modification of function, glusterd_store_volinfo_write in
glusterd-store.c, wherein a pointer of type
glusterd_volinfo_data_store_t is initialized for further
operation. Also, the buffer is emptied into the volfile before
it is freed.

4. Modification of function, glusterd_store_node_state_write in
glusterd-store.c, wherein the a pointer of type
glusterd_volinfo_data_store_t is initialized for further operations.
Also, the buffer is emptied into the volfile before it is freed.

5. Addition of enum into glusterd-mem-types.h

6. Modification of function, glusterd_store_options in
glusterd-store.c, wherein a pointer of type
glusterd_volinfo_data_store_t is initialized for further opertaions.
Also, the buffer is emptied into the volfile before it is freed.

Reasoning behind the approach:
1.Instead of a dynamic allocation of buffer or increasing the buffer
size with increased number of options, it, the current approach takes
a buffer of fixed size (VOLINFO_BUFFER_SIZE). Before any write into
the buffer, the size is checked and if it exceeds the available space,
the contents of the buffer are written to the file before copying new
contents. Dynamic allocation can lead to increased memory usage as one
doesn't know the number of options that could be added in time and may
go on to occupy more space than mandated.

2.The function dict_foreach is a generic function used across different
modules. It made sense not to change its implementation as it might
affect other Functionalities. Hence a structure was added which could
just be passed as one of the parameter to this function (as it takes
a void*).

3. Reduced number of system calls implies an increase in execution speed.
Also, these modified functions come into play whenever the volume is
started or modified.

4. The functions _storeslaves, _gd_store_rebalace_dict and
_store_global_opts were doing the same set of operations as that of
_storeopts except the checking for the key. This has been handled
with the help of a flag in the glusterd_volinfo_data_store_t structure.
This reduces the duplicate code present.

Signed-off-by: Srijan Sivakumar <ssivakum@redhat.com>
Change-Id: I22e6e91c78ed51e3a171482054d77bf793b9ab16
Fixes: #718

1 files changed, 124 insertions, 89 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 716eedba62b..e714fae41d3 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -660,85 +660,70 @@ out:
 }
 
 static int
-_storeslaves(dict_t *this, char *key, data_t *value, void *data)
-{
-    int32_t ret = 0;
-    gf_store_handle_t *shandle = NULL;
-    xlator_t *xl = NULL;
-
-    xl = THIS;
-    GF_ASSERT(xl);
-
-    shandle = (gf_store_handle_t *)data;
-
-    GF_ASSERT(shandle);
-    GF_ASSERT(shandle->fd > 0);
-    GF_ASSERT(shandle->path);
-    GF_ASSERT(key);
-    GF_ASSERT(value);
-    GF_ASSERT(value->data);
-
-    gf_msg_debug(xl->name, 0, "Storing in volinfo:key= %s, val=%s", key,
-                 value->data);
-
-    ret = gf_store_save_value(shandle->fd, key, (char *)value->data);
-    if (ret) {
-        gf_msg(xl->name, GF_LOG_ERROR, 0, GD_MSG_STORE_HANDLE_WRITE_FAIL,
-               "Unable to write into store"
-               " handle for path: %s",
-               shandle->path);
-        return -1;
-    }
-    return 0;
-}
-
-int
-_storeopts(dict_t *this, char *key, data_t *value, void *data)
+_storeopts(dict_t *dict_value, char *key, data_t *value, void *data)
 {
     int32_t ret = 0;
     int32_t exists = 0;
+    int32_t option_len = 0;
     gf_store_handle_t *shandle = NULL;
-    xlator_t *xl = NULL;
+    glusterd_volinfo_data_store_t *dict_data = NULL;
+    xlator_t *this = NULL;
 
-    xl = THIS;
-    GF_ASSERT(xl);
+    this = THIS;
+    GF_ASSERT(this);
 
-    shandle = (gf_store_handle_t *)data;
+    dict_data = (glusterd_volinfo_data_store_t *)data;
+    shandle = dict_data->shandle;
 
     GF_ASSERT(shandle);
     GF_ASSERT(shandle->fd > 0);
-    GF_ASSERT(shandle->path);
     GF_ASSERT(key);
     GF_ASSERT(value);
     GF_ASSERT(value->data);
 
-    if (is_key_glusterd_hooks_friendly(key)) {
-        exists = 1;
+    if (dict_data->key_check == 1) {
+        if (is_key_glusterd_hooks_friendly(key)) {
+            exists = 1;
 
-    } else {
-        exists = glusterd_check_option_exists(key, NULL);
+        } else {
+            exists = glusterd_check_option_exists(key, NULL);
+        }
     }
-
-    if (1 == exists) {
-        gf_msg_debug(xl->name, 0,
-                     "Storing in volinfo:key= %s, "
+    if (exists == 1 || dict_data->key_check == 0) {
+        gf_msg_debug(this->name, 0,
+                     "Storing in buffer for volinfo:key= %s, "
                      "val=%s",
                      key, value->data);
-
     } else {
-        gf_msg_debug(xl->name, 0, "Discarding:key= %s, val=%s", key,
+        gf_msg_debug(this->name, 0, "Discarding:key= %s, val=%s", key,
                      value->data);
         return 0;
     }
 
-    ret = gf_store_save_value(shandle->fd, key, (char *)value->data);
-    if (ret) {
-        gf_msg(xl->name, GF_LOG_ERROR, 0, GD_MSG_STORE_HANDLE_WRITE_FAIL,
-               "Unable to write into store"
-               " handle for path: %s",
-               shandle->path);
+    /*
+     * The option_len considers the length of the key value
+     * pair and along with that '=' and '\n'.
+     */
+    option_len = strlen(key) + strlen((char *)value->data) + 2;
+
+    if ((VOLINFO_BUFFER_SIZE - dict_data->buffer_len - 1) < option_len) {
+        ret = gf_store_save_items(shandle->fd, dict_data->buffer);
+        if (ret) {
+            gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_FILE_OP_FAILED, NULL);
+            return -1;
+        }
+        dict_data->buffer_len = 0;
+        dict_data->buffer[0] = '\0';
+    }
+    ret = snprintf(dict_data->buffer + dict_data->buffer_len, option_len + 1,
+                   "%s=%s\n", key, (char *)value->data);
+    if (ret < 0 || ret > option_len + 1) {
+        gf_smsg(this->name, GF_LOG_ERROR, EINVAL, GD_MSG_COPY_FAIL, NULL);
         return -1;
     }
+
+    dict_data->buffer_len += ret;
+
     return 0;
 }
 
@@ -1013,7 +998,7 @@ glusterd_store_create_snap_dir(glusterd_snap_t *snap)
     return ret;
 }
 
-int32_t
+static int32_t
 glusterd_store_volinfo_write(int fd, glusterd_volinfo_t *volinfo)
 {
     int32_t ret = -1;
@@ -1021,19 +1006,47 @@ glusterd_store_volinfo_write(int fd, glusterd_volinfo_t *volinfo)
     GF_ASSERT(fd > 0);
     GF_ASSERT(volinfo);
     GF_ASSERT(volinfo->shandle);
+    xlator_t *this = NULL;
+    glusterd_volinfo_data_store_t *dict_data = NULL;
+
+    this = THIS;
+    GF_ASSERT(this);
 
     shandle = volinfo->shandle;
+
+    dict_data = GF_CALLOC(1, sizeof(glusterd_volinfo_data_store_t),
+                          gf_gld_mt_volinfo_dict_data_t);
+    if (dict_data == NULL) {
+        gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_NO_MEMORY, NULL);
+        return -1;
+    }
+
     ret = glusterd_volume_exclude_options_write(fd, volinfo);
-    if (ret)
+    if (ret) {
         goto out;
+    }
+
+    dict_data->shandle = shandle;
+    dict_data->key_check = 1;
 
     shandle->fd = fd;
-    dict_foreach(volinfo->dict, _storeopts, shandle);
+    dict_foreach(volinfo->dict, _storeopts, (void *)dict_data);
+
+    dict_data->key_check = 0;
+    dict_foreach(volinfo->gsync_slaves, _storeopts, (void *)dict_data);
+
+    if (dict_data->buffer_len > 0) {
+        ret = gf_store_save_items(fd, dict_data->buffer);
+        if (ret) {
+            gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_FILE_OP_FAILED, NULL);
+            goto out;
+        }
+    }
 
-    dict_foreach(volinfo->gsync_slaves, _storeslaves, shandle);
     shandle->fd = 0;
 out:
-    gf_msg_debug(THIS->name, 0, "Returning %d", ret);
+    GF_FREE(dict_data);
+    gf_msg_debug(this->name, 0, "Returning %d", ret);
     return ret;
 }
 
@@ -1274,14 +1287,6 @@ out:
     return ret;
 }
 
-static int
-_gd_store_rebalance_dict(dict_t *dict, char *key, data_t *value, void *data)
-{
-    int fd = *(int *)data;
-
-    return gf_store_save_value(fd, key, value->data);
-}
-
 int32_t
 glusterd_store_node_state_write(int fd, glusterd_volinfo_t *volinfo)
 {
@@ -1289,6 +1294,12 @@ glusterd_store_node_state_write(int fd, glusterd_volinfo_t *volinfo)
     char buf[PATH_MAX];
     char uuid[UUID_SIZE + 1];
     uint total_len = 0;
+    glusterd_volinfo_data_store_t *dict_data = NULL;
+    gf_store_handle_t shandle;
+    xlator_t *this = NULL;
+
+    this = THIS;
+    GF_ASSERT(this);
 
     GF_ASSERT(fd > 0);
     GF_ASSERT(volinfo);
@@ -1328,14 +1339,33 @@ glusterd_store_node_state_write(int fd, glusterd_volinfo_t *volinfo)
     }
 
     ret = gf_store_save_items(fd, buf);
-    if (ret)
+    if (ret) {
         goto out;
+    }
 
     if (volinfo->rebal.dict) {
-        dict_foreach(volinfo->rebal.dict, _gd_store_rebalance_dict, &fd);
+        dict_data = GF_CALLOC(1, sizeof(glusterd_volinfo_data_store_t),
+                              gf_gld_mt_volinfo_dict_data_t);
+        if (dict_data == NULL) {
+            gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_NO_MEMORY, NULL);
+            return -1;
+        }
+        dict_data->shandle = &shandle;
+        shandle.fd = fd;
+        dict_foreach(volinfo->rebal.dict, _storeopts, (void *)dict_data);
+        if (dict_data->buffer_len > 0) {
+            ret = gf_store_save_items(fd, dict_data->buffer);
+            if (ret) {
+                gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_FILE_OP_FAILED,
+                        NULL);
+                goto out;
+                ;
+            }
+        }
     }
 out:
-    gf_msg_debug(THIS->name, 0, "Returning %d", ret);
+    GF_FREE(dict_data);
+    gf_msg_debug(this->name, 0, "Returning %d", ret);
     return ret;
 }
 
@@ -3343,20 +3373,6 @@ glusterd_store_set_options_path(glusterd_conf_t *conf, char *path, size_t len)
     snprintf(path, len, "%s/options", conf->workdir);
 }
 
-int
-_store_global_opts(dict_t *this, char *key, data_t *value, void *data)
-{
-    gf_store_handle_t *shandle = data;
-
-    if (gf_store_save_value(shandle->fd, key, (char *)value->data)) {
-        gf_msg(THIS->name, GF_LOG_ERROR, 0, GD_MSG_STORE_HANDLE_WRITE_FAIL,
-               "Unable to write into store handle for key : %s, value %s", key,
-               (char *)value->data);
-    }
-
-    return 0;
-}
-
 int32_t
 glusterd_store_options(xlator_t *this, dict_t *opts)
 {
@@ -3365,13 +3381,15 @@ glusterd_store_options(xlator_t *this, dict_t *opts)
     char path[PATH_MAX] = {0};
     int fd = -1;
     int32_t ret = -1;
+    glusterd_volinfo_data_store_t *dict_data = NULL;
 
     conf = this->private;
     glusterd_store_set_options_path(conf, path, sizeof(path));
 
     ret = gf_store_handle_new(path, &shandle);
-    if (ret)
+    if (ret) {
         goto out;
+    }
 
     fd = gf_store_mkstemp(shandle);
     if (fd <= 0) {
@@ -3379,13 +3397,30 @@ glusterd_store_options(xlator_t *this, dict_t *opts)
         goto out;
     }
 
+    dict_data = GF_CALLOC(1, sizeof(glusterd_volinfo_data_store_t),
+                          gf_gld_mt_volinfo_dict_data_t);
+    if (dict_data == NULL) {
+        gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_NO_MEMORY, NULL);
+        return -1;
+    }
+    dict_data->shandle = shandle;
     shandle->fd = fd;
-    dict_foreach(opts, _store_global_opts, shandle);
-    shandle->fd = 0;
+    dict_foreach(opts, _storeopts, (void *)dict_data);
+    if (dict_data->buffer_len > 0) {
+        ret = gf_store_save_items(fd, dict_data->buffer);
+        if (ret) {
+            gf_smsg(this->name, GF_LOG_ERROR, 0, GD_MSG_FILE_OP_FAILED, NULL);
+            goto out;
+        }
+    }
+
     ret = gf_store_rename_tmppath(shandle);
-    if (ret)
+    if (ret) {
         goto out;
+    }
 out:
+    shandle->fd = 0;
+    GF_FREE(dict_data);
     if ((ret < 0) && (fd > 0))
         gf_store_unlink_tmppath(shandle);
     gf_store_handle_destroy(shandle);