Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

librados: add map_object API #7749

Closed
wants to merge 2 commits into from

Conversation

zhouyuan
Copy link
Contributor

Ceph use crush algorithm to provide the mapping of objects to OSD servers.
However there's no public API for this instead you'll have to rely on the
command to monitor:

$ ceph osd map pool_name obj_name

The "ceph_get_osd_crush_location" method is made available via libcephfs
but not avaiale in librados.

This patch provides an API to render the object distribution layout in librados.
With this the application could choose which nodes to access, for load-balance
purpose.

Signed-off-by: Yuan Zhou yuan.zhou@intel.com

@zhouyuan
Copy link
Contributor Author

Hi @liewegas, sorry for the delay, this is the followup of #6222. Can you please kindly help to have a look?

@zhouyuan zhouyuan force-pushed the librados_map_object branch 2 times, most recently from 12f0b1c to 2809667 Compare February 23, 2016 12:57
int up_primary, acting_primary;
o.pg_to_up_acting_osds(pgid, &up, &up_primary, &acting, &acting_primary);
string upsetstr(up.begin(), up.end());
ldout(cct, 10) << "Object: " << obj_name << "up: " << upsetstr << ", p" << up_primary << dendl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't nee dto use the intermediate strings... there is an operator<< overload for most stl containers so you can just print it directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liewegas thanks! will do in next rev.

@liewegas
Copy link
Member

I don't really like the string-based return type for the C version. We originally started doing this due to Colin's paranoia about different allocators being used by the calling code and by librados. Is that a real concern? Switching to int* and having the caller free seems easiest. Alternatively, the caller could provide a max buffer size and allocate the vector themselves. Or we could make a rados_intvector_free that looks just like ceph_buffer_free. Just using a user-allocated buffer and max_size seems the simplest, though...

@liewegas liewegas self-assigned this Feb 26, 2016
@liewegas
Copy link
Member

@jdurgin thoughts?

@zhouyuan zhouyuan force-pushed the librados_map_object branch 3 times, most recently from 6ca08de to 7337d2b Compare March 9, 2016 06:12
@zhouyuan
Copy link
Contributor Author

zhouyuan commented Mar 9, 2016

@liewegas
Hi Sage, I've made some changes on the C API. Now it's using strcpy instead. Does this look better? Thanks!

@liewegas
Copy link
Member

liewegas commented Mar 9, 2016 via email

@jdurgin
Copy link
Member

jdurgin commented Mar 9, 2016

sure, and I just found a gh search operator (involves:username) so I don't lose track of this again

* @returns 0 on success, negative error code on failure
*/
CEPH_RADOS_API int rados_map_object(rados_t cluster, const char *obj_name,
char *up, size_t *up_size,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that you're having the caller allocate them, is there any reason not to use int* instead of char* for up and acting?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you're calling this a lot, avoiding the string conversion will be more efficient too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might want a version of this on IoCtx that reuses the locator (including namespace) set there

zhouyuan added 2 commits April 5, 2016 08:53
Ceph use crush algorithm to provide the mapping of objects to OSD servers.
However there's no public API for this instead you'll have to rely on the
command to monitor:

$ ceph osd map pool_name obj_name

The "ceph_get_osd_crush_location" method is made available via libcephfs
but not avaiale in librados.

This patch provides an API to render the object distribution layout in librados.
With this the application could choose which nodes to access, for load-balance
purpose.

The C++ API is:
map_object(const char *obj_name, std::vector<int>& up,
           std::vector<int>& acting, int64_t pool_id)

The C API is:
CEPH_RADOS_API int rados_map_object(rados_t cluster, const char *obj_name,
                                    int *up, size_t *up_size,
                                    int *acting, size_t *acting_size,
                                    int64_t pool_id);

With the new APIs we may get the data location(# of OSD) through the rados
instance.

Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
This patch adds the librados map_object API in python rados binding

Signed-off-by: Yuan Zhou <yuan.zhou@intel.com>
@zhouyuan zhouyuan force-pushed the librados_map_object branch from 7337d2b to a3e3116 Compare April 5, 2016 01:17
@zhouyuan
Copy link
Contributor Author

zhouyuan commented Apr 5, 2016

@jdurgin
Hi Josh, thanks for the review. I made some updates here but I'm not clear about "IoCtx that reuses the locator (including namespace) set there". Could you please kindly share more thoughts on this?

@jdurgin
Copy link
Member

jdurgin commented Apr 5, 2016

@zhouyuan rados objects are mapped to pgs based on 'namespace', 'locator', and 'object name', which are all strings.

An IoCtx in librados is stateful, and lets you set a namespace or locator to use for subsequent operations.
I was suggesting to add IoCtx::map_object() and rados_ioctx_map_object() as well, which would use the IoCtx's state to map the object based on the full information (with locator and namespace coming from IoCtxImpl->oloc.{key, nspace})

@jdurgin
Copy link
Member

jdurgin commented Apr 5, 2016

Looking again, I think it would make sense to have only the IoCtx APIs, and not the plain Rados/rados_ ones.

@zhouyuan
Copy link
Contributor Author

zhouyuan commented Apr 5, 2016

@jdurgin thanks for the quick review! I see your point now. Will move the APIs to IoCtx in next push.

@liewegas
Copy link
Member

liewegas commented May 3, 2016

@zhouyuan ping

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants