[sheepdog] [PATCH v3 3/3] sheep/http: add lock to protect container and object

[Robin Dong]

2013/12/12 Liu Yuan <namei.unix at gmail.com>

> On Wed, Dec 11, 2013 at 06:14:43PM +0800, Robin Dong wrote:
> > From: Robin Dong <sanbai at taobao.com>
> >
> > Add locks to protect containers and objects when users create/delete
> > containers or create/delete objects in the same time.
> >
> > Signed-off-by: Robin Dong <sanbai at taobao.com>
> > ---
> >  sheep/http/kv.c    | 252
> ++++++++++++++++++++++++++++++++---------------------
> >  sheep/http/swift.c |  10 ++-
> >  2 files changed, 162 insertions(+), 100 deletions(-)
> >
> > diff --git a/sheep/http/kv.c b/sheep/http/kv.c
> > index fb66dfa..4d690cb 100644
> > --- a/sheep/http/kv.c
> > +++ b/sheep/http/kv.c
> > @@ -218,22 +218,22 @@ static int kv_get_account(const char *account,
> uint32_t *nr_buckets)
> >
> >       ret = lookup_vdi(account, &account_vid);
> >       if (ret != SD_RES_SUCCESS)
> > -             return ret;
> > +             goto out;
> >
> >       /* read account vdi out */
> >       oid = vid_to_vdi_oid(account_vid);
> >       ret = sd_read_object(oid, (char *)&inode, sizeof(struct sd_inode),
> 0);
> >       if (ret != SD_RES_SUCCESS) {
> >               sd_err("Failed to read inode header %lx", oid);
> > -             return ret;
> > +             goto out;
> >       }
> >
> >       struct list_buckets_arg arg = {NULL, NULL, NULL, 0};
> >       traverse_btree(sheep_bnode_reader, &inode, list_buckets_cb, &arg);
> >       if (nr_buckets)
> >               *nr_buckets = arg.bucket_counter;
> > -
> > -     return SD_RES_SUCCESS;
> > +out:
> > +     return ret;
> >  }
> >
> >  int kv_read_account(const char *account, uint32_t *nr_buckets)
> > @@ -528,21 +528,22 @@ out:
> >  }
> >
> >  static int kv_get_bucket(struct sd_inode *account_inode, const char
> *account,
> > -                      const char *bucket)
> > +                      const char *bucket, uint32_t *account_vid)
>
> account_inode already has the account_vid, no? So we can remove
> account_vdi as
> parameter.
>

No, the account_vid will be set by lookup_vdi() below, so we can't remove
it.


>
> >  {
> >       char vdi_name[SD_MAX_VDI_LEN];
> >       uint64_t oid;
> > -     uint32_t account_vid, bucket_vid;
> > +     uint32_t bucket_vid;
> >       int ret;
> >
> > -     ret = lookup_vdi(account, &account_vid);
> > +     ret = lookup_vdi(account, account_vid);
> >       if (ret != SD_RES_SUCCESS) {
> >               sd_err("Failed to find account %s", account);
> >               return -1;
> >       }
> >
> > +     sys->cdrv->lock(*account_vid);
>
> Seems that you don't unlock after exit of this function?
>

We must lock the account_vid before read the accout_inode and unlock after
create(or delete) the container,
so we call lock() in kv_get_bucket() and call unlock() in the end
ofkv_create_bucket() and kv_delete_bucket().
If not so, the buckets will corrupt under parallely high pressure tests (
https://github.com/RobinDong/misc/blob/master/sheepdog_test/pswift.sh)


> Thanks
> Yuan
>
>


-- 
--
Best Regard
Robin Dong
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wpkg.org/pipermail/sheepdog/attachments/20131212/01692c25/attachment-0004.html>