[sheepdog] [PATCH] farm: parallize cluster-wide snapshot by work queue

Liu Yuan namei.unix at gmail.com
Thu May 30 15:21:05 CEST 2013


On 05/30/2013 08:23 PM, Kai Zhang wrote:
> Signed-off-by: Kai Zhang <kyle at zelin.io>
> ---
>  collie/farm/farm.c        |  184 ++++++++++++++++++++++++++++++++------------
>  collie/farm/farm.h        |   12 +--
>  collie/farm/object_tree.c |   15 +---
>  collie/farm/trunk.c       |   15 +---
>  4 files changed, 146 insertions(+), 80 deletions(-)
> 
> diff --git a/collie/farm/farm.c b/collie/farm/farm.c
> index cb4aaa9..bb7b36c 100644
> --- a/collie/farm/farm.c
> +++ b/collie/farm/farm.c
> @@ -29,6 +29,16 @@ struct vdi_entry {
>  };
>  static LIST_HEAD(last_vdi_list);
>  
> +struct snapshot_work {
> +	uint64_t oid;
> +	int nr_copies;
> +	unsigned char sha1[SHA1_LEN];
> +	struct strbuf *trunk_entries;
> +	struct work work;
> +};
> +struct work_queue *wq;
> +static uatomic_bool work_error;
> +
>  static struct vdi_entry *find_vdi(const char *name)
>  {
>  	struct vdi_entry *vdi;
> @@ -186,39 +196,6 @@ static int notify_vdi_add(uint32_t vdi_id, uint32_t nr_copies)
>  	return ret;
>  }
>  
> -static int fill_trunk_entry(uint64_t oid, int nr_copies,
> -			    void *buf, size_t size, void *data)
> -{
> -	int ret = -1;
> -
> -	struct strbuf *trunk_entries = data;
> -	struct trunk_entry new_entry = {};
> -	struct sha1_file_hdr hdr = { .priv = 0 };
> -	struct strbuf object_strbuf = STRBUF_INIT;
> -
> -	memcpy(hdr.tag, TAG_DATA, TAG_LEN);
> -	hdr.size = size;
> -
> -	strbuf_add(&object_strbuf, buf, size);
> -	strbuf_insert(&object_strbuf, 0, &hdr, sizeof(hdr));
> -
> -	if (sha1_file_write((void *)object_strbuf.buf,
> -			    object_strbuf.len,
> -			    new_entry.sha1) != 0)
> -		goto out;
> -
> -	new_entry.oid = oid;
> -	new_entry.nr_copies = nr_copies;
> -	strbuf_add(trunk_entries, &new_entry, sizeof(struct trunk_entry));
> -
> -	ret = 0;
> -out:
> -	if (ret)
> -		fprintf(stderr, "Fail to fill trunk entry\n.");
> -	strbuf_release(&object_strbuf);
> -	return ret;
> -}
> -
>  int farm_init(const char *path)
>  {
>  	int ret = -1;
> @@ -240,14 +217,78 @@ bool farm_contain_snapshot(uint32_t idx, const char *tag)
>  	return (get_trunk_sha1(idx, tag, trunk_sha1) == 0);
>  }
>  
> +static void save_object_main(struct work *work)
> +{
> +	void *sha1_buf, *data_buf;
> +	size_t sha1_size, data_size;
> +	struct snapshot_work *sw;
> +	struct sha1_file_hdr *hdr;
> +
> +	if (uatomic_is_true(&work_error))
> +		return;
> +
> +	sw = container_of(work, struct snapshot_work, work);
> +	data_size = get_objsize(sw->oid);
> +	sha1_size = data_size + sizeof(struct sha1_file_hdr);
> +	hdr = sha1_buf = xmalloc(sha1_size);
> +	data_buf = (char *)sha1_buf + sizeof(struct sha1_file_hdr);
> +
> +	if (sd_read_object(sw->oid, data_buf, data_size, 0, true) < 0)
> +		goto error;
> +
> +	memcpy(hdr->tag, TAG_DATA, TAG_LEN);
> +	hdr->size = data_size;
> +
> +	if (sha1_file_write(sha1_buf, sha1_size, sw->sha1) < 0)
> +		goto error;
> +
> +	goto out;
> +error:
> +	fprintf(stderr, "Fail to save object, oid %"PRIu64"\n",
> +		sw->oid);
> +	uatomic_set_true(&work_error);
> +out:
> +	free(sha1_buf);
> +}

It is more clear to put error handling after the normal return as:

out:
   clean_up_something();
   return success;
err:
   clean_up_something();
   return false;

Even though sometimes we duplicate some statements in the
clean_up_something(). GCC might do smarter optimization to remove the
duplicate statements sometimes, if not, we just trade several bytes to
get a better readability.

> +
> +static void save_object_done(struct work *work)
> +{
> +	struct trunk_entry entry;
> +	struct snapshot_work *sw = container_of(work, struct snapshot_work,
> +						work);
> +
> +	if (uatomic_is_true(&work_error))
> +		goto out;
> +
> +	entry.oid = sw->oid;
> +	entry.nr_copies = sw->nr_copies;
> +	memcpy(entry.sha1, sw->sha1, SHA1_LEN);
> +	strbuf_add(sw->trunk_entries, &entry, sizeof(struct trunk_entry));
> +out:
> +	free(sw);
> +}
> +
> +static int queue_save_snapshot_work(uint64_t oid, int nr_copies, void *data)
> +{
> +	struct snapshot_work *sw = xzalloc(sizeof(struct snapshot_work));
> +	struct strbuf *trunk_entries = data;
> +	sw->oid = oid;
> +	sw->nr_copies = nr_copies;
> +	sw->trunk_entries = trunk_entries;
> +	sw->work.fn = save_object_main;

We sometimes use 'main' to mean it is running in the main thread. So
we'd better have save_object_main as do_save_object. So load_object_main
-> do_load_object.

By the way, maybe we can read sha1 of object first to save duplicate
transfer of the objects that maps to the same sha1.

Thanks,
Yuan


More information about the sheepdog mailing list