Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

add workarounds for servers that don't paginate correctly #167

Merged
merged 3 commits into from
Oct 6, 2021

Conversation

vmpjdc
Copy link
Contributor

@vmpjdc vmpjdc commented Mar 18, 2021

Some Swift API implementations (I've observed this with Ceph
RADOSGW) can return fewer results than specified by the "limit"
parameter, even when we have not reached the end of the listing.

It's unclear to me from reading the API docs whether this is
a violation of the API specification, but since it happens in the
wild, it's best to be able to handle it.

One way of doing this is to simply keep fetching pages until we
receive an empty page. Another is to assume that pages within
a certain percentage of limit are not the last page. Given the
tradeoffs involved, let's support both.

@vmpjdc
Copy link
Contributor Author

vmpjdc commented Mar 18, 2021

I discovered this problem while trying to use rclone to copy data from an OpenStack Swift into a Ceph RADOSGW.

Listings of some RADOSGW containers would terminate after 1999 entries even though there were over 10,000 objects in them, and rclone copy would copy the same objects again and again on multiple runs.

With this change applied to a local build of rclone I'm now getting full listings and clean syncs with no spurious copies.

Copy link
Owner

@ncw ncw left a comment

Choose a reason for hiding this comment

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

This change will do one more API transaction than it needs to on conforming servers won't it?

So for your average directory listing this will now do two transactions rather than one.

I can't see a way round this in the protocol - can you?

@vmpjdc
Copy link
Contributor Author

vmpjdc commented Mar 25, 2021

It will indeed result in an extra transaction, and unfortunately I can't see another way around it. (The number of objects in a container can be looked up by issuing a HEAD request, but of course that's still an extra request, and racy to boot.)

Correcting myself, I found a separate page describing the Swift API's pagination and RADOSGW is clearly in the wrong, at least in terms of that document.

However, I also discovered that Swift's own Python client code (which I think we could probably call the reference client) doesn't implement pagination as described above when fetching full listings, rather just fetching new pages until it receives an empty one. (Link is into get_container; get_account does the same.)

The code dates from 2012, so perhaps this was implemented before API pagination was nailed down. Either way, swift list and get_container(..., full_listing=True, limit=1000) do not trip over this RADOSGW bug.

@ncw
Copy link
Owner

ncw commented Mar 25, 2021

One thing we could do is make a feature flag for this and only do the new behaviour if the feature flag is set.

So make a new flag in the Connection struct and check it to enable the new behaviour.

I have a feeling that this has already been reported as a radosgw bug - it would be worth searching their issue tracker to see.

I hate the idea of doubling the number of transactions for directory traversals - I can see that being very bad for performance.

@ncw
Copy link
Owner

ncw commented Mar 26, 2021

...or we could use some kind of heuristic - if we got more than 90% of the max listing then do an extra transaction just to check.

This would probably work quite well but has the potential to go wrong.

@vmpjdc
Copy link
Contributor Author

vmpjdc commented Mar 28, 2021

The 90% heuristic should work nicely, based on the lengths of the replies I'm getting from RADOSGW for my problematic buckets:

reply lengths: 1000 999 1000 1000 1000 1000 1000 1000 1000 1000 119
reply lengths: 1000 992 1000 1000 1000 1000 1000 935 1000 1000 257
reply lengths: 1000 1000 1000 1000 1000 975 1000 948
reply lengths: 953 1000 1000 1000 1000 1000 954 1000 1000 70
reply lengths: 1000 1000 1000 1000 998 15
reply lengths: 1000 1000 1000 1000 974 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 939 1000 1000 1000 1000 949 1000 1000 1000 644
reply lengths: 1000 1000 1000 1000 999 1000 1000 937 1000 1000 538
reply lengths: 1000 998 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 551
reply lengths: 1000 1000 1000 1000 1000 1000 1000 931 1000 986 1000 1000 1000 975 1000 989 1000 1000 1000 966 1000 998 921 994 1000 1000 973 58
reply lengths: 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 1000 976 1000 366
reply lengths: 1000 1000 1000 1000 1000 983 1000 1000 1000 1000 1000 1000 1000 517
reply lengths: 1000 1000 1000 984 1000 1000 971 1000 1000 401
reply lengths: 949 1000 1000 1000 1000 1000 1000 403
reply lengths: 1000 998 532
reply lengths: 951 1000 1000 1000 1000 1000 976 1000 877

I was not able to find a matching bug in the Ceph tracker, but I'll start a thread on ceph-users to confirm.

And I'll also take a look at implementing the flag and heuristic.

@ncw
Copy link
Owner

ncw commented Mar 31, 2021

The 90% heuristic should work nicely, based on the lengths of the replies I'm getting from RADOSGW for my problematic buckets:

useful data - thanks

Those missing items are probably filtered out items (eg deleted items) or something like that.

I was not able to find a matching bug in the Ceph tracker, but I'll start a thread on ceph-users to confirm.

If you find something out can you link it here?

And I'll also take a look at implementing the flag and heuristic.

:-)

@vmpjdc
Copy link
Contributor Author

vmpjdc commented Apr 6, 2021

Those missing items are probably filtered out items (eg deleted items) or something like that.

I'm reasonably certain that it's not deleted items, at least -- it's a fairly new cluster, and these buckets have only ever been written to by rclone copy.

I was not able to find a matching bug in the Ceph tracker, but I'll start a thread on ceph-users to confirm.

If you find something out can you link it here?

Most definitely!

And I'll also take a look at implementing the flag and heuristic.

:-)

Implemented, and from my testing with hacked rclone builds, the workarounds seem to perform as expected when enabled.

@vmpjdc vmpjdc changed the title use empty result as sentinel when fetching container listings add workarounds for servers that don't paginate correctly Apr 7, 2021
@vmpjdc
Copy link
Contributor Author

vmpjdc commented Apr 11, 2021

I forgot to mention I made a draft rclone PR that makes this change much easier to try out: rclone/rclone#5224

Copy link
Owner

@ncw ncw left a comment

Choose a reason for hiding this comment

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

Apologies for the delay!

This looks great now - I'll merge it

Thank you :-)

@ncw ncw merged commit fef4f1d into ncw:master Oct 6, 2021
vmpjdc added a commit to vmpjdc/rclone that referenced this pull request Oct 26, 2021
Ceph's Swift API emulation does not fully confirm to the API spec.
As a result, it sometimes returns fewer items in a container than
the requested limit, which according to the spec should means
that there are no more objects left in the container.  (Note that
python-swiftclient always fetches unless the current page is empty.)

This commit adds a pair of new Swift backend settings to handle this.

Set `fetch_until_empty_page` to true to always fetch another
page of the container listing unless there are no items left.

Alternatively, set `partial_page_fetch_threshold` to an integer
percentage.  In this case rclone will fetch a new page only when
the current page is within this percentage of the limit.

Swift API reference: https://docs.openstack.org/swift/latest/api/pagination.html

PR against ncw/swift with research and discussion: ncw/swift#167
vmpjdc added a commit to vmpjdc/rclone that referenced this pull request Dec 3, 2021
Ceph's Swift API emulation does not fully confirm to the API spec.
As a result, it sometimes returns fewer items in a container than
the requested limit, which according to the spec should means
that there are no more objects left in the container.  (Note that
python-swiftclient always fetches unless the current page is empty.)

This commit adds a pair of new Swift backend settings to handle this.

Set `fetch_until_empty_page` to true to always fetch another
page of the container listing unless there are no items left.

Alternatively, set `partial_page_fetch_threshold` to an integer
percentage.  In this case rclone will fetch a new page only when
the current page is within this percentage of the limit.

Swift API reference: https://docs.openstack.org/swift/latest/api/pagination.html

PR against ncw/swift with research and discussion: ncw/swift#167
ncw pushed a commit to rclone/rclone that referenced this pull request Jun 28, 2024
Ceph's Swift API emulation does not fully confirm to the API spec.
As a result, it sometimes returns fewer items in a container than
the requested limit, which according to the spec should means
that there are no more objects left in the container.  (Note that
python-swiftclient always fetches unless the current page is empty.)

This commit adds a pair of new Swift backend settings to handle this.

Set `fetch_until_empty_page` to true to always fetch another
page of the container listing unless there are no items left.

Alternatively, set `partial_page_fetch_threshold` to an integer
percentage.  In this case rclone will fetch a new page only when
the current page is within this percentage of the limit.

Swift API reference: https://docs.openstack.org/swift/latest/api/pagination.html

PR against ncw/swift with research and discussion: ncw/swift#167

Fixes #7924
ncw pushed a commit to rclone/rclone that referenced this pull request Jun 28, 2024
Ceph's Swift API emulation does not fully confirm to the API spec.
As a result, it sometimes returns fewer items in a container than
the requested limit, which according to the spec should means
that there are no more objects left in the container.  (Note that
python-swiftclient always fetches unless the current page is empty.)

This commit adds a pair of new Swift backend settings to handle this.

Set `fetch_until_empty_page` to true to always fetch another
page of the container listing unless there are no items left.

Alternatively, set `partial_page_fetch_threshold` to an integer
percentage.  In this case rclone will fetch a new page only when
the current page is within this percentage of the limit.

Swift API reference: https://docs.openstack.org/swift/latest/api/pagination.html

PR against ncw/swift with research and discussion: ncw/swift#167

Fixes #7924
Fornax96 pushed a commit to Fornaxian/rclone that referenced this pull request Jul 30, 2024
Ceph's Swift API emulation does not fully confirm to the API spec.
As a result, it sometimes returns fewer items in a container than
the requested limit, which according to the spec should means
that there are no more objects left in the container.  (Note that
python-swiftclient always fetches unless the current page is empty.)

This commit adds a pair of new Swift backend settings to handle this.

Set `fetch_until_empty_page` to true to always fetch another
page of the container listing unless there are no items left.

Alternatively, set `partial_page_fetch_threshold` to an integer
percentage.  In this case rclone will fetch a new page only when
the current page is within this percentage of the limit.

Swift API reference: https://docs.openstack.org/swift/latest/api/pagination.html

PR against ncw/swift with research and discussion: ncw/swift#167

Fixes rclone#7924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants