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

[Liu Yuan]

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