librados: remove legacy object listing API, clean up newer api #13149
Conversation
| /** | ||
| * @warning Deprecated: Use rados_nobjects_list_open() instead | ||
| */ | ||
| CEPH_RADOS_API int rados_objects_list_open(rados_ioctx_t io, |
There was a problem hiding this comment.
we are removing APIs from librados, so shall we take this chance to consider http://pad.ceph.com/p/librados3?
There was a problem hiding this comment.
thought we talked about making these return -ENOTSUPP instead?
|
as a follow-on we should switch rados.pyx's ObjectIterator to use the cursor methods too - it uses the nobjects api |
|
see 042b856
|
|
the new cursor methods are compatible with !sortbitwise clusters, so I think we should stick with nobjects for a few more releases (probably until we remove it).
|
| ldout(cct, 10) << " hobject sort order changed, restarting this pg" | ||
| << dendl; | ||
| list_context->cookie = collection_list_handle_t(); | ||
| list_context->pos = hobject_t( |
There was a problem hiding this comment.
shouldn't snapid be 0 for the min hobject_t() here and a couple places you construct hobject_t with CEPH_NOSNAP (-2)?
There was a problem hiding this comment.
Hmm.. possibly. It would only matter if you set the ioctx read snapid. The code to support this in the OSD is very slow (we have to load the object_info_t for the head and the relevant clone), so I'm wonder if we want to remove support for it altogether.
In the meantime, I guess we should just use the read snap id here?
There was a problem hiding this comment.
actually, looking at this more closely, the list_nobjects() Objecter interface doesn't have a way to specify a snap and never has. So CEPH_NOSNAP is correct in this case. If we wanted to change the API to support listing a snap we could (and IIRC there have been PRs in the past to do this) but until that happens i'm pretty sure NOSNAP is the right thing. In particular, the arg is "start after", and we give it the last item we got before, so we want it to be == (not <) that previous item.. if we change to 0 here i think we'll get dups on the request boundaries.
liewegas
changed the title
liewegas
changed the title
liewegas
force-pushed
the
wip-list-objects
branch
from
d0d78d2
to
370746b
Compare
|
retest this please |
| @@ -61,15 +61,6 @@ set_target_properties(ceph_test_rados_api_list PROPERTIES COMPILE_FLAGS | |||
| target_link_libraries(ceph_test_rados_api_list | |||
| rados_a global ${UNITTEST_LIBS} radostest) | |||
|
|
|||
| # ceph_test_rados_api_nlist | |||
There was a problem hiding this comment.
@liewegas shouldn't remove this test
|
The _list binary is switched to test the nlist api. the _nlist binary was
never run by the test scripts, AFAICS :(
|
|
@liewegas oic, will need to merge my new tests into _list |
Stop using current_pg as a position pointer; use the hobject_t cursor explicitly. We keep current_pg *only* for compatibility with !sortbitwise clusters, and we only use it when we get back MAX from a !sortbitwise OSD and need to determine where the start of the next PG is. In !sortbitwise mode we also have the legacy kludges to behave on PG split. Signed-off-by: Sage Weil <sage@redhat.com>
Yes, we send an extra 4 bytes per entry for the namespace, but who cares; this reduces the amount of code we need to maintain. Signed-off-by: Sage Weil <sage@redhat.com>
This is means we don't know how to list objects on pre-hammer OSDs. (The PGNLS op was added around a03f85a Signed-off-by: Sage Weil <sage@redhat.com>
These have been deprecated since giant. Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
Signed-off-by: Sage Weil <sage@redhat.com>
liewegas
force-pushed
the
wip-list-objects
branch
from
370746b
to
c1ff854
Compare
Also add some minimal documentation for the new and preferred API.
This adventure began by trying to clean up the internal Objecter list_nobjects() code, but in the end I ended up deprecating it (and removing the older legacy interface entirely). I'm not sure it's worth the churn on that code at all, given that all users should be moving to the new enumeration code anyway. Should I just drop that patch?