summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSrijan Sivakumar <ssivakum@redhat.com>2020-06-02 16:47:24 +0530
committerMOHIT AGRAWAL <moagrawa@redhat.com>2020-09-17 12:10:58 +0000
commit558fe62a28f05fab2a913e9e9ed1206ed2d28d71 (patch)
tree6384d7c0373da74454b1567b7cd8597aff244240
parent5348292bf0fd79035b5817915922133cee8eb785 (diff)
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
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-mem-types.h1
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-messages.h2
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-store.c213
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-store.h15
4 files changed, 140 insertions, 91 deletions
diff --git a/xlators/mgmt/glusterd/src/glusterd-mem-types.h b/xlators/mgmt/glusterd/src/glusterd-mem-types.h
index 17052ce..d7257e1 100644
--- a/xlators/mgmt/glusterd/src/glusterd-mem-types.h
+++ b/xlators/mgmt/glusterd/src/glusterd-mem-types.h
@@ -27,6 +27,7 @@ typedef enum gf_gld_mem_types_ {
gf_gld_mt_mop_stage_req_t,
gf_gld_mt_probe_ctx_t,
gf_gld_mt_glusterd_volinfo_t,
+ gf_gld_mt_volinfo_dict_data_t,
gf_gld_mt_glusterd_brickinfo_t,
gf_gld_mt_peer_hostname_t,
gf_gld_mt_defrag_info,
diff --git a/xlators/mgmt/glusterd/src/glusterd-messages.h b/xlators/mgmt/glusterd/src/glusterd-messages.h
index c089179..ba4e34d 100644
--- a/xlators/mgmt/glusterd/src/glusterd-messages.h
+++ b/xlators/mgmt/glusterd/src/glusterd-messages.h
@@ -447,4 +447,4 @@ GLFS_MSGID(
"Failed to allocate memory or get serialized length of dict"
#define GD_MSG_GET_XATTR_FAIL_STR "Failed to get extended attribute"
-#endif /* !_GLUSTERD_MESSAGES_H_ */ \ No newline at end of file
+#endif /* !_GLUSTERD_MESSAGES_H_ */
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 716eedb..e714fae 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);
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.h b/xlators/mgmt/glusterd/src/glusterd-store.h
index 0407054..83f4df0 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.h
+++ b/xlators/mgmt/glusterd/src/glusterd-store.h
@@ -29,7 +29,7 @@ typedef enum glusterd_store_ver_ac_ {
} glusterd_volinfo_ver_ac_t;
#define UUID_SIZE 36
-
+#define VOLINFO_BUFFER_SIZE 4093
#define GLUSTERD_STORE_UUID_KEY "UUID"
#define GLUSTERD_STORE_KEY_VOL_TYPE "type"
@@ -112,6 +112,19 @@ typedef enum glusterd_store_ver_ac_ {
#define GLUSTERD_STORE_KEY_GANESHA_GLOBAL "nfs-ganesha"
+/*
+ * The structure is responsible for handling the parameter for writes into
+ * the buffer before it is finally written to the file. The writes will be
+ * of the form of key-value pairs.
+ */
+struct glusterd_volinfo_data_store_ {
+ gf_store_handle_t *shandle; /*Contains fd and path of the file */
+ int16_t buffer_len;
+ char key_check; /* flag to check if key is to be validated before write*/
+ char buffer[VOLINFO_BUFFER_SIZE];
+};
+typedef struct glusterd_volinfo_data_store_ glusterd_volinfo_data_store_t;
+
int32_t
glusterd_store_volinfo(glusterd_volinfo_t *volinfo,
glusterd_volinfo_ver_ac_t ac);