Skip to contentSkip to conversation

Speedup pvs

PATCH New Zdenek Kabelac 1 participant 3 messages Mar 26, 2012

lore
Share & tools
lore
#0 lore raw

cover Speedup pvs

Proposals to fight slow execution of commands likes pvs. Which is especially visible with lvmetad.

Zdenek Kabelac (2):
Proposal to speedup pvs ver.1
Avoid reparsing same metadata

 lib/config/config.c              |   10 ++-
 lib/config/config.h              |    3 +-
 lib/format_text/import.c         |   19 +++++-
 lib/metadata/metadata-exported.h |   10 +++
 lib/metadata/metadata.c          |   30 ++++++---
 tools/toollib.c                  |  128 ++++++++++++++++++++++++++++----------
 6 files changed, 153 insertions(+), 47 deletions(-)
Signature · 1 line
1.7.9.3
#1 lore raw

PATCH 1/2 Proposal to speedup pvs ver.1

First version of acceleration of pvs command. Introduce 'smarter' scan_pvs() function which is able to run process_pv_fn_t for each pv (essentially similar to process_single_pv used in toollib - maybe we move typedef into lib/metadata/metadata-exported.h and use the same type?)

For _process_all_pvs - it's not maintaining hashtable of used PVs.

For _process_all_devs - maintains used PVs hash table.

NOTICE: moving lvmcache_seed_infos_from_lvmetad() used only in _process_all_devs() to _get_pvs - IMHO it's bug - but I'm not so sure about the logic.

NOTICE2: currently patch DOES NOT accelerate pvs if devices are given on command line - this will be added if we will consider this is the way we want to go.

To give the illusion of speed increase:

300PVs creating one 1VG:

without patch & lvmetad 28 minutes.
with patch & lvmetad 5 seconds
with patch & without lvmetad 1second

---
 lib/metadata/metadata-exported.h |    5 ++
 lib/metadata/metadata.c          |   30 ++++++---
 tools/toollib.c                  |  128 ++++++++++++++++++++++++++++----------
 3 files changed, 121 insertions(+), 42 deletions(-)
lib/metadata/metadata-exported.h +5 -0
index 8ebf487..10cd910 100644
@@ -413,6 +413,11 @@ struct physical_volume *pv_read(struct cmd_context *cmd, const char *pv_name,
 				int warnings,
 				int scan_label_only);
 struct dm_list *get_pvs(struct cmd_context *cmd);
+typedef int (*process_pv_fn_t) (struct cmd_context *cmd,
+				struct volume_group *vg,
+				struct physical_volume *pv,
+				void *handle);
+int scan_pvs(struct cmd_context *cmd, process_pv_fn_t fn, void *handle);
 
 /*
  * Add/remove LV to/from volume group
lib/metadata/metadata.c +22 -8
index bd13a23..7d65d8b 100644
@@ -3693,7 +3693,8 @@ struct dm_list *get_vgids(struct cmd_context *cmd, int include_internal)
 	return lvmcache_get_vgids(cmd, include_internal);
 }
 
-static int _get_pvs(struct cmd_context *cmd, int warnings, struct dm_list **pvslist)
+static int _get_pvs(struct cmd_context *cmd, int warnings, struct dm_list **pvslist,
+		    process_pv_fn_t process_pv, void *handle)
 {
 	struct str_list *strl;
 	struct dm_list * uninitialized_var(results);
@@ -3705,6 +3706,7 @@ static int _get_pvs(struct cmd_context *cmd, int warnings, struct dm_list **pvsl
 	int old_pvmove;
 
 	lvmcache_label_scan(cmd, 0);
+	lvmcache_seed_infos_from_lvmetad(cmd);
 
 	if (pvslist) {
 		if (!(results = dm_pool_alloc(cmd->mem, sizeof(*results)))) {
@@ -3727,8 +3729,6 @@ static int _get_pvs(struct cmd_context *cmd, int warnings, struct dm_list **pvsl
 	init_pvmove(1);
 	dm_list_iterate_items(strl, vgids) {
 		vgid = strl->str;
-		if (!vgid)
-			continue;	/* FIXME Unnecessary? */
 		consistent = 0;
 		if (!(vgname = lvmcache_vgname_from_vgid(NULL, vgid))) {
 			stack;
@@ -3741,9 +3741,14 @@ static int _get_pvs(struct cmd_context *cmd, int warnings, struct dm_list **pvsl
 		if (!consistent)
 			log_warn("WARNING: Volume Group %s is not consistent",
 				 vgname);
-
-		/* Move PVs onto results list */
-		if (pvslist)
+		if (process_pv) {
+			dm_list_iterate_items(pvl, &vg->pvs)
+				if (!process_pv(cmd, vg, pvl->pv, handle)) {
+					release_vg(vg);
+					return_0;
+				}
+		} else if (pvslist)
+			/* Copy PVs onto results list */
 			dm_list_iterate_items(pvl, &vg->pvs) {
 				if (!(pvl_copy = _copy_pvl(cmd->mem, pvl))) {
 					log_error("PV list allocation failed");
@@ -3764,11 +3769,20 @@ static int _get_pvs(struct cmd_context *cmd, int warnings, struct dm_list **pvsl
 	return 1;
 }
 
+int scan_pvs(struct cmd_context *cmd, process_pv_fn_t process_pv, void *handle)
+{
+	if (!_get_pvs(cmd, 1, NULL, process_pv, handle))
+		return_0;
+
+	return 1;
+}
+
+/* FIXME: Convert users to use more efficient scan_pvs() if possible */
 struct dm_list *get_pvs(struct cmd_context *cmd)
 {
 	struct dm_list *results;
 
-	if (!_get_pvs(cmd, 1, &results))
+	if (!_get_pvs(cmd, 1, &results, NULL, NULL))
 		return NULL;
 
 	return results;
@@ -3776,7 +3790,7 @@ struct dm_list *get_pvs(struct cmd_context *cmd)
 
 int scan_vgs_for_pvs(struct cmd_context *cmd, int warnings)
 {
-	return _get_pvs(cmd, warnings, NULL);
+	return _get_pvs(cmd, warnings, NULL, NULL, NULL);
 }
 
 int pv_write(struct cmd_context *cmd __attribute__((unused)),
tools/toollib.c +94 -34

176 lines not on this page · raw message

Signature · 1 line
1.7.9.3
#2 lore raw

PATCH 2/2 Avoid reparsing same metadata

In reply to Zdenek Kabelac

When reading VG - we read mda from some PV - do all the validation, and then we read another mda from next PV and the same thing again. But in case - we could trust checksum and length - it seems just wasting CPU (i.e. using 300PVs for a VG would lead to create and destroy 300 config trees....)

---
 lib/config/config.c              |   10 ++++++++--
 lib/config/config.h              |    3 ++-
 lib/format_text/import.c         |   19 +++++++++++++++++--
 lib/metadata/metadata-exported.h |    5 +++++
 4 files changed, 32 insertions(+), 5 deletions(-)
lib/config/config.c +8 -2
index 67a8b04..532812c 100644
@@ -189,7 +189,8 @@ int override_config_tree_from_string(struct cmd_context *cmd,
 
 int config_file_read_fd(struct dm_config_tree *cft, struct device *dev,
 			off_t offset, size_t size, off_t offset2, size_t size2,
-			checksum_fn_t checksum_fn, uint32_t checksum)
+			checksum_fn_t checksum_fn, uint32_t checksum,
+			int skip_parse)
 {
 	char *fb, *fe;
 	int r = 0;
@@ -230,6 +231,11 @@ int config_file_read_fd(struct dm_config_tree *cft, struct device *dev,
 		goto out;
 	}
 
+	if (skip_parse) {
+		r = 1;
+		goto out;
+	}
+
 	fe = fb + size + size2;
 	if (!dm_config_parse(cft, fb, fe))
 		goto_out;
@@ -273,7 +279,7 @@ int config_file_read(struct dm_config_tree *cft)
 	}
 
 	r = config_file_read_fd(cft, cf->dev, 0, (size_t) info.st_size, 0, 0,
-				(checksum_fn_t) NULL, 0);
+				(checksum_fn_t) NULL, 0, 0);
 
 	if (!cf->keep_open) {
 		if (!dev_close(cf->dev))
lib/config/config.h +2 -1
index d789ade..7eb56f0 100644
@@ -30,7 +30,8 @@ typedef uint32_t (*checksum_fn_t) (uint32_t initial, const uint8_t *buf, uint32_
 struct dm_config_tree *config_file_open(const char *filename, int keep_open);
 int config_file_read_fd(struct dm_config_tree *cft, struct device *dev,
 			off_t offset, size_t size, off_t offset2, size_t size2,
-			checksum_fn_t checksum_fn, uint32_t checksum);
+			checksum_fn_t checksum_fn, uint32_t checksum,
+			int skip_parse);
 	int config_file_read(struct dm_config_tree *cft);
 int config_write(struct dm_config_tree *cft, const char *file,
 		 int argc, char **argv);
lib/format_text/import.c +17 -2
index 3d2f496..26f6cb9 100644
@@ -89,6 +89,7 @@ struct volume_group *text_vg_import_fd(struct format_instance *fid,
 	struct volume_group *vg = NULL;
 	struct dm_config_tree *cft;
 	struct text_vg_version_ops **vsn;
+	int skip;
 
 	_init_text_import();
 
@@ -98,13 +99,21 @@ struct volume_group *text_vg_import_fd(struct format_instance *fid,
 	if (!(cft = config_file_open(file, 0)))
 		return_NULL;
 
+	skip = fid->vg && (fid->mda_checksum == checksum) &&
+		(fid->mda_size == (size + size2));
+
 	if ((!dev && !config_file_read(cft)) ||
-	    (dev && !config_file_read_fd(cft, dev, offset, size,
-					 offset2, size2, checksum_fn, checksum))) {
+	    (dev && !config_file_read_fd(cft, dev, offset, size, offset2,
+					 size2, checksum_fn, checksum, skip))) {
 		log_error("Couldn't read volume group metadata.");
 		goto out;
 	}
 
+	if (skip) {
+		vg = fid->vg;
+		goto out;
+	}
+
 	/*
 	 * Find a set of version functions that can read this file
 	 */
@@ -119,6 +128,12 @@ struct volume_group *text_vg_import_fd(struct format_instance *fid,
 		break;
 	}
 
+	if (vg && (!fid->vg || (vg->seqno > fid->vg->seqno))) {
+		fid->vg = vg;
+		fid->mda_size = (size + size2);
+		fid->mda_checksum = checksum;
+	}
+
       out:
 	config_file_destroy(cft);
 	return vg;
lib/metadata/metadata-exported.h +5 -0
index 10cd910..7d1812c 100644
@@ -233,6 +233,11 @@ struct format_instance {
 	struct dm_list metadata_areas_ignored;
 	struct dm_hash_table *metadata_areas_index;
 
+	/* Speedup parser - avoid repeateadly parse same mda content */
+	struct volume_group *vg;
+	uint32_t mda_checksum;
+	size_t mda_size;
+
 	void *private;
 };
 
Signature · 1 line
1.7.9.3