Re: librados compound operations

From: Colin McCabe <cmccabe@alumni.cmu.edu>
To: Sage Weil <sage@newdream.net>
Cc: ceph-devel@vger.kernel.org
Subject: Re: librados compound operations
Date: Mon, 28 Mar 2011 18:29:30 -0700	[thread overview]
Message-ID: <AANLkTin9P9UMSmKPmVwMCSBjPMQ6zGKVamzUbQaPmkVJ@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1103281539580.17288@cobra.newdream.net>

On Mon, Mar 28, 2011 at 3:46 PM, Sage Weil <sage@newdream.net> wrote:
> Below is a patch that wraps the internal Objecter compound ObjectOperation
> so that we can send compound operations to the OSDs via librados.
>
> The internal ObjectOperationImpl ends up being unnecessary; I just cast it
> to the internal type directly.
>
> One weird thing I noticed is with the constructor.  In Rados:: did
>
> +    static ObjectOperation *operation_create();
>
> and the user then deletes it when they're done (after using it for one or
> more calls to operate() or aio_operate()).  Is that the the approach we
> want?  Because right above that in the header is
>
>     int ioctx_create(const char *name, IoCtx &pioctx);
>

I second TV's argument that returning a detailed error code is better
than returning NULL or not-NULL.

I also don't like calling conventions that always use dynamic memory
allocation. Even on a shiny new 16-core system, there is still a
global mutex around new()/free(). I also like putting stuff on the
stack and using RAII. (I haven't looked at this new code enough to see
whether RAII makes sense here though.)

If we do go with the new/free based approach, I guess we need another
function to call free() on it, because the library user might be using
a different libc.

cheers,
Colin
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html